[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