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

Artem Belevich via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Feb 15 10:45:57 PST 2023


tra added a comment.

It's OK to keep both 32-bit and 64-bit variants. Even if we already generate correct code for 32-bit variant, it's still useful to make sure we do not regress in the future.



================
Comment at: llvm/test/CodeGen/NVPTX/bfe.ll:34
+
+; CHECK: bfe3
+define i32 @bfe3(i32 %a) {
----------------
`CHECK-LABEL:`

I'd also give it a more meaningful name. `no_bfe_on_32bit_oveflow`.


================
Comment at: llvm/test/CodeGen/NVPTX/bfe.ll:36
+define i32 @bfe3(i32 %a) {
+; CHECK-NOT: bfe %r{{[0-9]+}}, %r{{[0-9]+}}, 31, 4
+; CHECK: shr
----------------
This would never match as the actual instruction would be `bfe.u32`


================
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
----------------
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`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117118



More information about the llvm-commits mailing list