[llvm-commits] InReg patch for review

[Evan Cheng]

Comments:

diff -r bdd0e76350e4 lib/Target/PowerPC/PPCISelLowering.cpp
--- a/lib/Target/PowerPC/PPCISelLowering.cpp	Tue Jan 23 13:34:14 2007  
+0300
+++ b/lib/Target/PowerPC/PPCISelLowering.cpp	Tue Jan 23 01:12:46 2007  
+0300
@@ -1360,9 +1360,10 @@ static SDOperand LowerCALL(SDOperand Op,
      // On PPC64, promote integers to 64-bit values.
      if (isPPC64 && Arg.getValueType() == MVT::i32) {
-      unsigned ExtOp = ISD::ZERO_EXTEND;
-      if (cast<ConstantSDNode>(Op.getOperand(5+2*i+1))->getValue())
-        ExtOp = ISD::SIGN_EXTEND;
+      unsigned Flags =
+        dyn_cast<ConstantSDNode>(Op.getOperand(5+2*i+1))->getValue();
+      unsigned ExtOp = (Flags & 1) ? ISD::SIGN_EXTEND :  
ISD::ZERO_EXTEND;
+
        Arg = DAG.getNode(ExtOp, MVT::i64, Arg);
      }

Please use cast<> not dyn_cast<> here.

    // Add one result value for each formal argument.
    std::vector<MVT::ValueType> RetVals;
+  unsigned j = 0;
    for (Function::arg_iterator I = F.arg_begin(), E = F.arg_end();  
I != E; ++I) {
      MVT::ValueType VT = getValueType(I->getType());
+    bool isInReg = FTy->paramHasAttr(++j,  
FunctionType::InRegAttribute);
+    unsigned Flags = (isInReg << 1);

+  // Handle regparm attribute
+  std::vector<bool> ArgInRegs(NumArgs, false);
+  if (!isVarArg) {
+    for (unsigned i = 0; i<NumArgs; ++i) {
+      unsigned Flags = cast<ConstantSDNode>(Op.getOperand(3+i))- 
 >getValue();
+      if ((Flags >> 1) & 1)
+        ArgInRegs[i] = true;
+    }
+  }

Please add getter / setter routines for these attributes.

In LowerFastCCArguments():

-      if (ObjectVT == MVT::i64 && ObjIntRegs) {
-        SDOperand ArgValue2 = DAG.getLoad(Op.Val->getValueType(i),  
Root, FIN,
-                                          NULL, 0);
-        ArgValue = DAG.getNode(ISD::BUILD_PAIR, MVT::i64, ArgValue,  
ArgValue2);
-      } else
-        ArgValue = DAG.getLoad(Op.Val->getValueType(i), Root, FIN,  
NULL, 0);
+      ArgValue = DAG.getLoad(Op.Val->getValueType(i), Root, FIN,  
NULL, 0);
+

I am not following. How are you passing i64 values? It has to take 2  
registers, right?

-    Entry.isSigned = false;
+    Entry.isSigned = false; Entry.isInReg = false;
      Args.push_back(Entry);
      // Extend the unsigned i8 argument to be an int value for the  
call.
      Entry.Node = DAG.getNode(ISD::ZERO_EXTEND, MVT::i32,  
Op.getOperand(2));
      Entry.Ty = IntPtrTy;
-    Entry.isSigned = false;
+    Entry.isSigned = false; Entry.isInReg = false;

Please separate each statement to its own line. :-)

The rest looks good! Thanks.

Evan

On Jan 23, 2007, at 5:13 AM, Anton Korobeynikov wrote:

> Hello, Everyone.
>
> Attached patch will implement "inreg" function argument attribute,  
> which
> is used to tell backend "place this argument in register, if  
> possible".
> The needed changes in x86 backend were made as well.
>
> This also allowed to merge C & StdCall CCs and Fast & FastCall CCs
> inside x86 backend.
>
> This patch is first step for supporting gcc's "regparm" function
> attribute.
>
> -- 
> With best regards, Anton Korobeynikov.
>
> Faculty of Mathematics & Mechanics, Saint Petersburg State University.
>
> <huge.diff>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits