[PATCH] D66991: [PowerPC] Fix SH field overflow issue

[Jinsong Ji via Phabricator via llvm-commits]

jsji requested changes to this revision.
jsji added a reviewer: PowerPC.
jsji added a comment.
This revision now requires changes to proceed.

Can you please use MIR test instead. Thanks.



================
Comment at: llvm/lib/Target/PowerPC/PPCInstrInfo.cpp:3592
         } else {
           // Left shifts use (N, 63-N), right shifts use (64-N, N).
+          uint64_t SH = ShAmt == 0 ? 0 : RightShift ? 64 - ShAmt : ShAmt;
----------------
Comments should be updated too. 


================
Comment at: llvm/test/CodeGen/PowerPC/convert-rr-to-ri-instrs.ll:3
+target datalayout = "e-m:e-i64:64-n32:64"
+target triple = "powerpc64le-unknown-linux-gnu"
+
----------------
Remove this line as well.


================
Comment at: llvm/test/CodeGen/PowerPC/convert-rr-to-ri-instrs.ll:15
+; instead.
+; We don't use MIR tests because llc doesn't serialize IsSSA property properly
+; so it doesn't do the conversion
----------------
run-pass may not trigger the assert, as we don't call the InstPrinters.
But I think you should still be able to run MIR test and trigger the assert with `start-before`.
eg:
```
$ llc -O3 -stop-before ppc-mi-peepholes ../llvm-project/llvm/test/CodeGen/PowerPC/convert-rr-to-ri-instrs.ll -o t.mir
$ llc -O3 -start-before ppc-mi-peepholes t.mir
llc: .../llvm/lib/Target/PowerPC/MCTargetDesc/PPCInstPrinter.cpp:327: void llvm::PPCInstPrinter::printU5ImmOperand(const llvm::MCInst*, unsigned int, llvm::raw_ostream&): Assertion `Value <= 31 && "Invalid u5imm argument!"' failed.
Stack dump:
0.      Program arguments: llc -O3 -start-before ppc-mi-peepholes t.mir 
1.      Running pass 'Function Pass Manager' on module 't.mir'.
2.      Running pass 'Linux PPC Assembly Printer' on function '@special_right_shift32_0'
...
Aborted (core dumped)
```


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

https://reviews.llvm.org/D66991