dberlin added a comment.

In https://reviews.llvm.org/D31885#730936, @rjmccall wrote:

> In https://reviews.llvm.org/D31885#730920, @dberlin wrote:
>
> > Just so i understand: Ignoring everything else (we can't actually make 
> > likelyalias work, i think the code in the bugs makes that very clear),
>
>
> None of the code in the bugs I've seen makes that very clear, but perhaps I 
> just missed the compelling example.
>
> > you also believe we should effectively pessimize every other language that 
> > generates correct TBAA info for LLVM  and will now no longer get optimized 
> > well because we've decided not to have clang emit TBAA metadata only in the 
> > cases where  it actually wants TBAA applied?
>
> Are you under the impression that I'm proposing something new and that TBAA 
> does not currently defer to BasicAA?


Well, actually, chandler changed how it works, so it doesn't actually work 
quite the way it used to but it has the same effect ;)
(It used to be explicit deferral, it is not explicit, neither depends on the 
other any more.  It just happens to be how the default AA pipeline is ordered 
right now)
I'm quite aware of the machinations of AA in LLVM :)

I'm saying "this approach does not work well now" (ie see bugs) , and i don't 
think continuing to try to make the contract even *more* incestuous is better 
than trying to make it *less*.

I see literally no advantage to doing so, and plenty of disadvantage 
(pessimization of other languages, continuation of a pipeline ordering that 
requires overrides, etc)

IE this is a thing we should be fixing, over time, to be a cleaner and better 
design, that does not make clang special here, and fixes these bugs.
Others generate tbaa metadata where they want it, and avoid it where they want 
basicaa to make a decision.

I have trouble seeing, why, if folks are willing to do that work, one would be 
opposed to it.
You seem to claim better optimization opportunity.
I ran numbers.
Across all things in llvm's benchmark suite, disabling unions entirely for tbaa 
causes no regressions on x86 (I could try more platforms if you like).

I also  ran it across 50 third party packages and spec2006 that use strict 
aliasing (These are just those things i could quickly test that had perf 
benchmarks)

If it really caused any real perf loss, we could always define metadata to say 
these accesses are noalias if we can't prove they are mustalias.
We could even use that metadata to explicitly try to have basicaa try much 
harder than it does now to prove must-aliasing (for example, ignore depth 
limits), since it would likely be too expensive to try it for everything.

as an aside, I do not believe you could reasonably apply this to unions  
because it's not possible to tell people what to do differently when it's *llvm 
ir* that has the issue.

IE i can't reasonably tell users "please stop doing things that don't generate 
explicit enough IR".
Especially when those answers change with inlining decisions, etc.

If the frontend did it, i would likely have something reasonably to tell them.


Repository:
  rL LLVM

https://reviews.llvm.org/D31885



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to