[PATCH] D109234: [PGO] Change ThinLTO test for targets with loop unrolling disabled

[Sherwin via Phabricator via cfe-commits]

sherwin-dc added inline comments.


================
Comment at: clang/test/CodeGen/pgo-sample-thinlto-summary.c:25
 
 // Checks if loop unroll is invoked by normal compile, but not thinlto compile.
 // SAMPLEPGO-LABEL: define {{(dso_local )?}}void @unroll
----------------
tejohnson wrote:
> It seems this test is explicitly checking for loop unrolling, so I'm not sure if it will fundamentally work the same if loop unrolling is off. Do you know why the below checking that "loop unroll is invoked by normal compile, but not thinlto compile" is not failing due to your target's change?
> 
> I wonder if this test should be converted somehow to an LLVM test.
Thats a good question. I guess I was too focused on the failing test below.

In the above file `samplepgo_nounroll.ll` I had ran this test on just the x86 backend without thinlto and with loop unrolling disabled in clang, but the loop still unrolled and the test passed. (Same story with the custom backend I am working on)

After changing the loop iterations from 2 to 3 then the test was failing which should have been the correct behavior with loop unrolling disabled. Not 100% sure on this but it looks as if a SimplifyCFG pass was unrolling the loop when it was 2 iterations, but not 3. So it looks like this needs to be changed if testing whether the loop-unroll pass was used.

Knowing this, I'm not sure what the next step would be. I dont know if any of the upstream backends had disabled loop unrolling, or how this could be tested in a target-independent way. (I'm quite new to LLVM)



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109234