[PATCH] D128911: Emit table lookup from TargetLowering::expandCTTZ()

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:7984
+      CA, getPointerTy(TD),
+      TD.getPrefTypeAlign(VT.getTypeForEVT(*DAG.getContext())));
+  Align Alignment = cast<ConstantPoolSDNode>(CPIdx)->getAlign();
----------------
Replace VT.getTypeForEVT(*DAG.getContext()) with `CA->getType()`


================
Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:7986
+  Align Alignment = cast<ConstantPoolSDNode>(CPIdx)->getAlign();
+  SDValue Load = DAG.getLoad(
+      VT, dl, DAG.getEntryNode(),
----------------
What happens if you always create a zextload even if the type isn't legal?


================
Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:7989
+      DAG.getMemBasePlusOffset(CPIdx, Lookup, dl), PtrInfo, Alignment);
+  SDValue ZExtOrTrunc = DAG.getZExtOrTrunc(Load, dl, VT);
+  if (isTypeLegal(VT))
----------------
This ZextOrTrunc doesn't do anything, the Load already has VT as its ValueType.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:7994
+    LoadSDNode *LD = cast<LoadSDNode>(ZExtOrTrunc);
+    return DAG.getExtLoad(
+        ISD::ZEXTLOAD, dl, VT, DAG.getEntryNode(),
----------------
The `VT` being passed to the operand named `MemVT` needs to be MemVT.


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

https://reviews.llvm.org/D128911