[PATCH] D56838: [CGP] Check for existing inttotpr before creating new one

[Hal Finkel via Phabricator via llvm-commits]

hfinkel accepted this revision.
hfinkel added a comment.
This revision is now accepted and ready to land.

In D56838#1362440 <https://reviews.llvm.org/D56838#1362440>, @rtereshin wrote:

> In D56838#1361579 <https://reviews.llvm.org/D56838#1361579>, @hfinkel wrote:
>
> > Can we run EarlyCSE after CGP (instead of trying to do these kinds of point fixes)?
>
>
> Hi Hal,
>
> Thank you for looking into this!
>
> That's a great suggestion! Originally I just eyeballed it as too expensive compile-time wise for the little effect that is achieved here.
>
> This time around with you pointing it out, I performed the actual measurements for a very large suite of shaders (the downstream target in question is a GPU). I see about 1.0 (+/-0.3)% increase in overall compile time (the part of it that happens at an application run-time) on average. The generated code quality improvement that comes out of this may be important, but it doesn't trigger often enough to justify 1% compile time increase across the board.
>
> Thanks,
> Roman


Fair enough. Thanks for checking.

I have a comment about the test case, but otherwise LGTM.



================
Comment at: test/Transforms/CodeGenPrepare/X86/sink-addrmode-cse-inttoptrs.ll:19
+; CGP-NOT: = inttoptr
+; LSV-NOT: = load float
+entry:
----------------
Please check for the expected result, not just the absence of the scalar load.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56838/new/

https://reviews.llvm.org/D56838