[PATCH] D138551: [PowerPC] Mask constant operands in bit permutation calculation

Mon Feb 6 22:50:08 PST 2023

tingwang added inline comments.

================
Comment at: llvm/lib/Target/PowerPC/PPCISelDAGToDAG.cpp:1487
+        // sld takes 7 bits, slw takes 6.
+        unsigned ShiftAmt = V.getConstantOperandVal(1) & ((NumBits << 1) - 1);

----------------
If I understand correctly, `ShiftAmt` is unsigned so a "negative value" means very large shift amount here. Can we simply cap `ShiftAmt` to be no larger than `NumBits`, and then seems additional boundary checks are not necessary?

================
Comment at: llvm/test/CodeGen/PowerPC/pr59074.ll:1
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=powerpc64le-unknown-linux-gnu -mcpu=pwr7 < %s | FileCheck %s --check-prefix=LE64
----------------
lkail wrote:
> Any idea why it doesn't crash on `pwr8`?
I think the DAGTypeLegalizer changed direction for this case on pwr8, for example:
https://github.com/llvm/llvm-project/blob/d18523c0430cee513a07b5b2a89ad1adb43c91f1/llvm/lib/Target/PowerPC/PPCISelLowering.cpp#L1059

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138551