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

Quentin Colombet qcolombet at apple.com
Mon Nov 3 10:32:11 PST 2014


Hi Ahmed,

I am wondering if you did not miss to include some files in the diff. I may be wrong but I would have expected to see some definitions of DAG nodes for SDIVREM8_SEXT_HREG and UDIVREM8_ZEXT_HREG.

Other comments inlined.

Thanks,
-Quentin

================
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);
----------------
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.

================
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');
     }
----------------
Now that you are done, does this debug line still make sense?

================
Comment at: test/CodeGen/X86/divrem8_ext.ll:1
@@ +1,2 @@
+; RUN: llc < %s | FileCheck %s
+target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
----------------
Do a x86_64 run and a i386 to test the change in the condition of the lowering of U/SREM.

http://reviews.llvm.org/D6064






More information about the llvm-commits mailing list