Live data from Hacker News

54-line if condition in gcc's reload.c

github.com

51–60 of 95 posts

Re: 54-line if condition in gcc's reload.c

#52
post #10

Earlier quoted context omitted.

It's been getting the job done for decades, in perhaps the most popular compiler of all time. I'm willing to trust the maintainers' judgement on this one. If they'd spent all their time needlessly fixing what ain't broke, gcc would've gone the way of GNU/Hurd.

> It's been getting the job done for decades, in perhaps the most popular compiler of all time. I'm willing to trust the maintainers' judgement on this one. I don't trust anyone - myself included - writing code 1/10th as convoluted, age-of-product be damned. Code is not wine, old doesn't mean good. Neither do the maintainers, methinks: Looking at blame shows some refactoring. > If they'd spent all their time needless…

If you've read some of the other posts, this whole file will hopefully be removed sometime in the future. For x86/x86_64 this file isn't of any use.

Edit: also reading the wiki, they are fully aware how bad this code is.

Re: 54-line if condition in gcc's reload.c

#53
post #43

Earlier quoted context omitted.

I prefer the operators at the start because the start is less jagged, so the operators don't get spread out so much and stand out more. Usually I only put them at the end of a line in languages that do implicit line endings, and will false-positive when you move the operator to the next line (i.e. javascript and visual basic).

Javascripr lets you put the binary operators in the next line if you want. It won't insert a semicolon there: http://blog.izs.me/post/2353458699/an-open-letter-to-javascr... I would say that the most common problem is having a semicolon not being inserted if you start a line with `(` or `[`. In practice, the only time when a semicolon gets inserted where it shouldn't is when returning an object literal.

Jshint still complains about it even though it's technically okay.

Re: 54-line if condition in gcc's reload.c

#55
post #22

Why put logical operator at the start and not the end of each line? I.e., this style (used in this case) && (CONSTANT_P (SUBREG_REG (in)) || GET_CODE (SUBREG_REG (in)) == PLUS || strict_low || (((REG_P (SUBREG_REG (in)) versus this style: (CONSTANT_P (SUBREG_REG (in)) || GET_CODE (SUBREG_REG (in)) == PLUS || strict_low || (((REG_P (SUBREG_REG (in)) && I don't have a personal preference here, just looking for any prac…

I used to use the latter, in both code and maths, but I have switched to the former. My main reason is that having the operators (logical and otherwise) at the start of the line means they're more likely to be in the same horizontal position, so it's easier to see which lines are continuations of the previous ones.

It is also easier to comment out lines of code if you need to test something. This is especially useful in SQL, where you can quickly -- a condition or a column, without editing commas etc on other lines, eg

SELECT

    some_col

    , another_col

    --, and_another

    , and_more
FROM

    blah
EDIT: too bad formatting is screwed up :/

Re: 54-line if condition in gcc's reload.c

#56

#ifdef should be shot

There are definitely times when it's overused, and times when it could be factored out. However, when you're dealing with the low level stuff a compiler that targets multiple architectures has to, using preprocessor defines can make things less maddening. GCC targets architectures that have all sorts of weird quirks regarding register locations and visibility, memory models, etc, that need to be accounted for. Without those #ifdefs, the code could very well be uglier with redundant checks, even uglier if statements, etc.

Re: 54-line if condition in gcc's reload.c

#57
Please note that:

1) The goals of Reload are very complex.

http://gcc.gnu.org/wiki/reload

"Reload does everything, and probably no one exactly knows how much that is. But to give you some idea:

Spill code generation

Instruction/register constraint validation

Constant pool building

Turning non-strict RTL (Register Transfer Language, a very low level intermediate representation used in the backends of GCC) into strict RTL (doing more of the above in evil ways).

Register elimination--changing frame pointer references to stack pointer references

Reload inheritance--essentially a builtin CSE (Common Subexpression Elimination) pass on spill code"

Reload achieved them for the last 25 years(!)

2) There are more modern approaches to reach such goals, but knowing 1) it is a lot of work before the goals can be achieved by some alternative code for all platforms (I don't know how far the developers got at the moment)

3) "Local Register Allocator Project" presentation by Vladimir Makarov, working for RedHat:

http://gcc.gnu.org/wiki/cauldron2012?action=AttachFile&do=ge...

Re: 54-line if condition in gcc's reload.c

#58
post #23

Earlier quoted context omitted.

Probably Git? You know that's been around much longer than Github has, right.

Of course, but even without checking Wikipedia, I remember a time not too terribly long ago when git, and even svn, didn't exist.

And are pretty sure that GCC was around at that point!

Re: 54-line if condition in gcc's reload.c

#59
The rest of that file also has a ton of redundancies and verboseness its in logic that could be simplified considerably; e.g. I see this pattern a lot (428 ~ 435):

    x && y || !x && z
In this absence of side-effects, this basically implements a 2-input multiplexer and is identical to

    x ? y : z
In the 54-line condition the first obvious thing I'd factor out is SUBREG_REG(in) and GET_MODE(SUBREG_REG(in)), and then work out what else is duplicated from there. Here's my attempt at making this a little more readable. It's only 2 lines less, but this gets rid of all the repeated uppercase:

http://pastebin.com/9zpr7Cd5

Re: 54-line if condition in gcc's reload.c

#60
post #8

No wonder llvm is getting popular (I realize that there are other reasons besides codebase quality).

Compilers are complex, and these conditionals had to be evaluated in some way. Sure the author could have split it up into multiple if statements, but would have that really helped?

`if` statements are for the 80s, we live in in the 90s and have objects, inheritance and C++. The conditions can be implicit in the objects.
Post reply on HN