[PATCH] D34141: [X86] Recognize constant arrays with special values and replace loads from it with subtract and shift instructions, which then will be replaced by X86 BZHI machine instruction.

[michael zuckerman via Phabricator via llvm-commits]

m_zuckerman added inline comments.


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:32498
+static SDValue combineAndLoadToBZHI(SDNode *Node, SelectionDAG &DAG,
+    const X86Subtarget &Subtarget) {
+  MVT VT = Node->getSimpleValueType(0);
----------------
align the line


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:32521
+    if (const GetElementPtrInst *GEP = dyn_cast<GetElementPtrInst>(MemOp)) {
+      if (GlobalVariable *GV = dyn_cast<GlobalVariable>(GEP->getOperand(0))) {
+        if (GV->isConstant() && GV->hasDefinitiveInitializer()) {
----------------
Can you explain why the element must to be a global variable? 


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:32526
+          Type *Ty = Init->getType();
+          if (!isa<ConstantDataArray>(Init) ||
+              !Ty->getArrayElementType()->isIntegerTy() ||
----------------
  # please clarify this if (add comments) 
  # Why you need also the isa<ConstantDataArray>(Init) ,is it not the same as GV->isConstant()? 


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:32548
+
+          // Do the transformation (For 32-bit type):
+          // -> (and (load arr[idx]), inp)
----------------
What about 64 bit? 


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:32552
+          //    that will be replaced with one bzhi instruction.
+          SDValue Inp = (i == 0) ? Node->getOperand(1) : Node->getOperand(0);
+          SDValue SizeC = DAG.getConstant(VT.getSizeInBits(), dl, VT);
----------------
You can write 
 SDValue Inp = Node->getOperand(1-i);


https://reviews.llvm.org/D34141