[PATCH] D33715: [PPC] exploit rotate-left-then-mask-insert instructions for bitfield insert

[Tony Jiang via Phabricator via llvm-commits]

jtony added inline comments.


================
Comment at: lib/Target/PowerPC/MCTargetDesc/PPCMCTargetDesc.h:89
     Val = ~Val; // invert mask
-    if (isShiftedMask_32(Val)) {
+    IsShiftedMask = Is64Bit ? isShiftedMask_64(Val):
+                              isShiftedMask_32(Val);
----------------
Missing space before `:`.  Actually, we may  want to put this short ternary expression in one line.


================
Comment at: lib/Target/PowerPC/PPCISelDAGToDAG.cpp:2932
   case ISD::OR: {
-    if (N->getValueType(0) == MVT::i32)
-      if (tryBitfieldInsert(N))
-        return;
+    if ((N->getValueType(0) == MVT::i32 && tryRotateThenMaskInsert<unsigned>(N)) ||
+        (N->getValueType(0) == MVT::i64 && tryRotateThenMaskInsert<uint64_t>(N)))
----------------
Like Nemanja have mentioned  above in his comment at line 1190 (lib/Target/PowerPC/AsmParser/PPCAsmParser.cpp). It may be more clear if we use `uint32_t` instead of `unsigned` here since we are using `uint64_t` for the 64 bit. Or we could also use `unsigned long long` instead of `uint64_t` to match the 32 bit type `unsigned`.  Personally, I would like we use this type pairs consistently (either `uint64_t`, `uint32_t`   or   `unsigned`, `unsigned long long` is fine), but it is up to you.


https://reviews.llvm.org/D33715