[PATCH] [X86] 8bit divrem: Improve codegen for AH register extraction.

Ahmed Bougacha ahmed.bougacha at gmail.com
Mon Nov 3 11:25:56 PST 2014


Hi Quentin,

I added 32-bit checks.
However, for the node definitions, are you talking about TableGen? If so, I didn't add those because I don't expect the added node types to be ever referenced in TableGen. I can still add them for consistency though, if you'd like.

Thanks for the review!

-Ahmed

================
Comment at: lib/Target/X86/X86ISelDAGToDAG.cpp:2545
@@ -2541,6 +2544,3 @@
     // that regard, this can be, and should be, removed.
-    if (HiReg == X86::AH && Subtarget->is64Bit() &&
-        !SDValue(Node, 1).use_empty()) {
-      SDValue Result = CurDAG->getCopyFromReg(CurDAG->getEntryNode(), dl,
-                                              X86::AX, MVT::i16, InFlag);
-      InFlag = Result.getValue(2);
+    if (HiReg == X86::AH && !SDValue(Node, 1).use_empty()) {
+      SDValue AHCopy = CurDAG->getRegister(X86::AH, MVT::i8);
----------------
qcolombet wrote:
> You slightly change the condition here by removing Subtarget->is64Bit().
> I believe this is correct, but you need to add a test case for that.
Right, I didn't explicitly check 32-bit. Added in the test file.

================
Comment at: lib/Target/X86/X86ISelDAGToDAG.cpp:2574
@@ -2562,1 +2573,3 @@
+      ReplaceUses(SDValue(Node, 1), Result);
+      DEBUG(dbgs() << "=> "; Result.getNode()->dump(CurDAG); dbgs() << '\n');
     }
----------------
qcolombet wrote:
> Now that you are done, does this debug line still make sense?
This is modeled after the debug outputs in the other parts of the code (for instance line 2582).

http://reviews.llvm.org/D6064






More information about the llvm-commits mailing list