On Mon, 18 Nov 2019, Jeff Law wrote:
> On 11/18/19 3:37 AM, Richard Biener wrote:
> > On Mon, 18 Nov 2019, Jakub Jelinek wrote:
> >
> >> On Mon, Nov 18, 2019 at 11:07:22AM +0100, Richard Biener wrote:
> >>> On Mon, 18 Nov 2019, Jakub Jelinek wrote:
> >>>
> >>>> On Mon, Nov 18, 2019 at 10:44:14AM +0100, Richard Biener wrote:
> >>>>> The following adjusts the find_base_{term,value} heuristic when
> >>>>> looking through ANDs to the case where it is obviously not
> >>>>> aligning a base (when the LSB is set).
> >>>>
> >>>> What is specific about the LSB? I mean & 2 is obviously not an aligning
> >>>> AND either.
> >>>
> >>> It aligns 0x1 and 0x3 ;)
> >>>
> >>>> Shouldn't we recurse only if the CONST_INT_P operand has
> >>>> some set bits followed by clear bits, i.e. after the != 0 check
> >>>> compute ctz_hwi and verify that INTVAL >> ctz == -1?
> >>>
> >>> I thought of more advanced heuristics but I guess that
> >>> any contiguous set of bits with LSB unset might be aligning
> >>> if the programmer knows upper bits are zero anyways? So I fear
> >>> the == -1 test would not work reliable?
> >>
> >> I'd say once a user does & 0x1ff8 or similar, he is doing something weird,
> >> and not recursing is the conservatively correct answer (or maybe it isn't?
> >> Aren't there cases where we punt if from a binary op we get base terms from
> >> both sides and just use one if we get it only from one side? If so,
> >> we might need to have some kind of annotated return, this could have a base
> >> term but please don't use it).
> >
> > Yes, we might run into the "wrong" one via binary op handling, so
> > there isn't really a conservative answer here :/
> >
> >> I guess the only non-weird case would be if the target has weird pointer
> >> sizes and only has larger or smaller ints, say 24-bit pointer, and 8/16/32
> >> integers, so the aligning then could be & 0xfffff8 etc.
> >
> > Yeah. Or the weird ones are exposed by nonzero bits "optimizations"
> > of constants.
> IIRC all the low order bitmask handling for pointers was primarily to
> benefit the Alpha. I think over time we saw it was helpful for
> varargs/stdarg, but that's about it. I'd be surprised if it's all that
> important to dig deeply into addresses of this form.
The whole find_base_{value,term} thing is most important for
stack accesses which we otherwise can't handle well in aliasing
and code effects mostly materialize in DSE. See my analysis
in PR49330. But the code is really broken and we're clawing
to the extra DSE in exchange for these wrong-code issues... :/
IMHO the appropriate way to go is to for example apply the
patch from comment#18 and try to fix code quality regressions
in a different way. Unfortunately the way RTL represents
stack accesses and implications for aliasing is a mystery to me
so I can't help much there - still I have received only push-back
in making find_base_{value,term} correct in the last 10 years
(well, it feels like 10 years).
After all, it's only about one wrong-code bug caused by this
per year making it into our bugzilla.
Richard.
