On Wed, 20 Nov 2019, Jeff Law wrote:
> On 11/19/19 12:42 AM, Richard Biener wrote:
> > 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... :/
> I'd rather fix the wrong-code issues :-)
>
> Note that REG_POINTER should already be conservative, if it isn't, then
> that needs to be fixed. Some backends certainly depend on anything with
> REG_POINTER actually being a valid pointer.
>
> For this RTX in c#18:
>
> > (plus:DI (reg:DI 83 [ d.0_2 ])
> > (symbol_ref:DI ("y") [flags 0x2] <var_decl 0x7ffff7fefb40 y>))
>
> I don't see how (reg 83) could ever be marked as REG_POINTER. It's an
> index, not a pointer. I would _not_ expect the result (assuming its
> stored in a REG) to be marked with REG_POINTER either since we don't
> know if the addition of (reg 83) creates a result outside the object
> (without additional information not in the BZ).
Sure, but we're not being called with this but with RTL
expanded via cselib usually:
(plus:SI (plus:SI (and:SI (symbol_ref:SI ("*.LANCHOR0") [flags 0x182])
(const_int 3 [0x3]))
(value:SI 8:8 @0x3089c18/0x30f5e40))
(const_int -8 [0xfffffffffffffff8]))
so there are no REGs involved on the "subexpressions" we could
check REG_POINTER on ... and in fact the symbol_ref _is_
a pointer! It's just not something we should consider the
base of the memory reference. We're talking about sth like
*(&a + ((uintptr_t)&b - (uintptr_t)&a))
where it's nearly impossible to tell (on RTL, if the address
parts are not split to separate insns and thus REG_POINTER is
applicable) what object we are looking at for aliasing purposes.
The appropriate way here is to not to too clever things with
the address but instead rely on MEM_EXPR here - unfortunately
that is non-existent for spill slots which is where most of
the regressions with removing the code appear.
Richard.
--
Richard Biener <rguent...@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
