[PATCH] D113351: [DAG] Add BuildVectorSDNode::getConstantRawBits helper

[Phoebe Wang via Phabricator via llvm-commits]

pengfei added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp:10932-10933
+
+  if (!isConstant())
+    return false;
+
----------------
Move ahead?


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp:10942
+      for (unsigned J = 0; J != Scale; ++J) {
+        SDValue Op = getOperand((I * Scale) + (IsLE ? J : (Scale - J - 1)));
+        if (Op.isUndef())
----------------
Do we need to check `IsLE` for `I`?


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp:10951
+            CInt ? CInt->getAPIntValue().zextOrTrunc(SrcEltSizeInBits)
+                 : CFP->getValueAPF().bitcastToAPInt(),
+            J * SrcEltSizeInBits);
----------------
Do we need to `zextOrTrunc` it?


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp:10973-10979
+      if (CInt) {
+        RawBits = CInt->getAPIntValue().extractBits(DstEltSizeInBits,
+                                                    J * DstEltSizeInBits);
+      } else {
+        RawBits = CFP->getValueAPF().bitcastToAPInt().extractBits(
+            DstEltSizeInBits, J * DstEltSizeInBits);
+      }
----------------
Maybe compiler can do so, but I think it's better to hoist the code outside the loop, e.g.
```
APInt SrcBits;
if (CInt)
  SrcBits = CInt->getAPIntValue();
else
  SrcBits = CFP->getValueAPF().bitcastToAPInt();

for (unsigned J = 0; J != Scale; ++J)
  RawBitElements[(I * Scale) + (IsLE ? J : (Scale - J - 1))] = SrcBits.extractBits(DstEltSizeInBits, J * DstEltSizeInBits);
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113351