Re: genattrab.c generate switch

[Jesper Broge JÃrgensen <jesperbroge at gmail dot com>]

On 01/02/16 20:19, Patrick Palka wrote:
> On Tue, Jan 12, 2016 at 7:53 PM, Jesper Broge JÃrgensen
> <jesperbroge@gmail.com> wrote:
> > Hello
> >
> > genattrab.c can generate if statements that have very deep bracket nesting
> > causing clang to produce errors (when target=arm-none-eabi) as explained at
> > https://gcc.gnu.org/ml/gcc/2014-05/msg00032.html
> > At the above link it was suggested that genattrab.c generated a switch
> > statement instead. I have made a patch that does just that.=
> Have you considered first implementing the other suggestion in that
> thread -- to avoid emitting the redundant parentheses in a consecutive
> chain ||s and &&s? That kind of fix would be simpler and would fix the
> compilation issue all the same, right?
I think it makes more sense to have the special case logic as close to 
where you know that you have to handle it eg. in write_attr_set() 
instead of in the more general purpos write_test_expr() (which is the 
where the parentheses is inserted). Also creating a different flow 
control structure (writing a switch instead of an if statement) makes it 
clear that we are handling an edge case. Also most of the logic needed 
is in the new function check_attr_set_switch() that is used to check if 
the expression is indeed a chain of || operators which would have to be 
done either way, so not much simplification would be gained.
write_test_expr() also makes the promise (in form of an inline comment) 
that parentheses are inserted to avoid worrying about operator 
precedence, having a special case that does not insert parentheses would 
break that promise.
Lastly i would also guess that a switch statement could be better 
optimized by the compiler but one would have to inspect the assembly 
output which i have not done so that is guess-work.