[PATCH] D117118: [NVPTX] Fix shr/and pair replace with bfe

[Artem Belevich via Phabricator via llvm-commits]

tra added a comment.

> What do you think about this approach instead of just simplifying the test?

SGTM.



================
Comment at: llvm/test/CodeGen/NVPTX/bfe.ll:46
+define i64 @bfe4(i64 %a) {
+; CHECK-NOT: bfe %r{{[0-9]+}}, %r{{[0-9]+}}, 63, 3
+; CHECK: shr
----------------
gmirazchiyski wrote:
> tra wrote:
> > This will also never match.  The instruction will be `bfe.u64` and the registers are 64-bit `%r*d*`.
> > 
> > Please run the tests with unmodified llvm and make sure it does fail when it detects bfe. 
> > 
> > As it happens, it will still fail when it would fail to find `shr`, but it would still be great to correctly test for what we intended to test.
> > As an alternative, you could simplify it all to `CHECK-NOT: bfe`
> > This will also never match. The instruction will be bfe.u64 and the registers are 64-bit %r*d*.
> Good catch!
> Yes, the expression also wasn't correct as you pointed out. I checked exactly what to match the produced `bfe` to and it has to be as follows:
> ```
> ; CHECK-NOT: bfe.u64 %rd{{[0-9]+}}, %rd{{[0-9]+}}, 63, 3
> ```
> 
> > Please run the tests with unmodified llvm and make sure it does fail when it detects bfe.
> I have run those and it does fail when `bfe` is detected. Without the llvm changes, `bfe.u64` is produced and unfortunately failed by reporting it's not being able find `shr` rather than because the `bfe` instruction was present.
> 
> That is not great because we're not communicating the issue too well in case this ever regresses in the future. This is because `CHECK`s seem to take precedence over `CHECK-NOT`s.
>  This is because CHECKs seem to take precedence over CHECK-NOTs.

I guess it's a fundamental difference between proving that something exists and proving that something does not exist.
Proving a negative is hard as it has to be done on all possible locations we're interested in, while a positive check needs to match only once, anywhere.
So, in order for a negative checker to work, we need to define the area which we'll be searching. That area is determined by the surrounding positive checks, so they have to be matched first.

I usually suggest using `-NOT` checks in a separate test run. Mixing them with positive checks is often tricky in non-obvious ways.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117118