Re: [PATCH] Fix PR63148 - two choices

[Jeff Law <law at redhat dot com>]

On 09/04/14 06:11, Richard Biener wrote:
> The following two patches each fix PR63148, a wrong-code issue
> caused by bogus array indices a-la
> &global_data.b[(sizetype) i + 536870911] which have a correct
> address when lowered but bogus index.
>
> The case in question can be mitigated by disabling folding of
> (sizetype) i * 8 + 4294967288 to ((sizetype) i + 536870911) * 8
> but that makes the real error - the existence of
> try_move_mult_to_index - just harder to trigger (I have already
> removed all similar code over the years).
>
> So the second variant is removing try_move_mult_to_index.
>
> The wrong-code is a regression from 4.7 which means a fix
> for the branches would be nice as well.
>
> Removing try_move_mult_to_index makes us correctly detect
> the dependence and not vectorize the testcase while only
> doing the mitigation makes dependence analysis fail and
> we create a runtime alias check (which would be a regression
> as well here - 4.7 also didn't vectorize this by detecting
> the dependence).
>
> I'm not sure which patch has the least side-effects on the
> branches.
>
> Sofar I have only fully tested removing try_move_mult_to_index
> on trunk which has some fallout that I have fixed and some
> fallout that should be addressed as followup.  The patch
> contains three new testcases from PR19807 we didn't add
> and for which we regressed two with 4.8 already (-2 and -3)
> and they still fail after the patch.
>
> Patch 2 is bootstrapped and tested on x86_64-unknown-linux-gnu,
> patch 1 is in testing.
>
> I have a strong preference to have patch 2 on trunk.
>
> Any preferences for the branches?
>
> Thanks,
> Richard.
>
>
> 2014-09-04  Richard Biener  <rguenther@suse.de>
>
> 	PR middle-end/63148
> 	* fold-const.c (fold_plusminus_mult_expr): Avoid case
> 	producing invalid array indexes when the multiplication
> 	is stripped.
>
> 	* gcc.dg/vect/pr63148.c: New testcase.
Given the pain we've had through the years with these kinds of 
out-of-bounds indices which are compensated for elsewhere in the address 
computation, I'm all for making them go away.

As two examples of where this matters, the PA port does its segment 
selection on just the base register, not the entire effective address. 
Thus when we muck around with indices like this, the base register has 
the compensation code and thus may result in selecting a different 
segment than would be the case if we looked at the entire effective address.

One of the mn103 parts has a similar problem, but it was actually a 
processor bug -- it was supposed to work correctly and select from the 
entire effective address, but it was broken in the hardware and we've 
had the "joys" of working around that errata in the compiler.

The argument for keeping this kind of braindamage has that it's allowed 
for more efficient code for things like virtual origin array addressing 
in Ada.  But it's been nothing but pain.

Jeff