[PATCH] D17588: Fix tests that used CHECK-NEXT-NOT and CHECK-DAG-NOT

Chad Rosier via llvm-commits llvm-commits at lists.llvm.org
Thu Feb 25 11:27:19 PST 2016


mcrosier accepted this revision.
mcrosier added a reviewer: mcrosier.
mcrosier added a comment.
This revision is now accepted and ready to land.

LGTM.


================
Comment at: test/Transforms/LoopVectorize/PowerPC/large-loop-rdx.ll:17
@@ -16,1 +16,3 @@
+; CHECK-NOT: fadd
+; CHECK-SAME: >
 
----------------
probinson wrote:
> mcrosier wrote:
> > probinson wrote:
> > > mcrosier wrote:
> > > > Why is the CHECK-SAME needed here?
> > > Using CHECK-SAME documents that the = and > are expected to be on the same line, thus achieving the effect that 'fadd' is also not on that line, i.e. that the sequence of 'fadd' instructions has ended.  Given how IR syntax works this could be a CHECK rather than CHECK-SAME and it would have the same effect.  I just think it's clearer with CHECK-SAME.  If you disagree I can change it.
> > AFAICT, the test is just checking that we generate exactly 12 fadd instructions.  Given that I think you could just replace
> > 
> > CHECK-NEXT-NOT: fadd
> > 
> > with
> > 
> > CHECK-NOT: fadd
> > CHECK: ret
> > 
> > IMO, that's a bit easier to understand.  What do you think, Paul?
> I tried that first, and it didn't work, because the test actually is not checking that there are exactly 12 fadd instructions.  It is checking that the first fadd instruction begins a sequence of exactly 12 fadd instructions.  There are other fadd instructions later on, after some other intervening stuff.
> 
> Now, if the intent of the test is that there are *only* 12 fadd instructions, that's different, but that's not how it was written originally.
Thanks for the clarification.


http://reviews.llvm.org/D17588





More information about the llvm-commits mailing list