[llvm-commits] [PATCH] Add the byval attribute and a bit more

[Chris Lattner]

On Jul 5, 2007, at 10:59 AM, Rafael Espindola wrote:

> The first email bounced, so lets try again.
>
> This patch implements the first part of the proposed solution to bug
> 1521. It adds the byval attribute and propagates it to DAG level.
>
> The patch also changes the matching code for CCIfType, so that no
> backend matches a byval argument. Instead, a backend should be
> modified to use a CCIfStruct (see the X86 backend). For now CCIfStruct
> is just a placeholder that aborts at run time.
>
> Then plan now is to implement a CCIfStruct that is common to all
> backends and just reproduces the same behavior of llvm-gcc. Later on
> we can change this to allow for backend specific code to actually fix
> the bug.
>
> Sounds like the right direction? OK to commit to trunk?

The patch looks good.  Some questions/comments:

+++ include/llvm/CodeGen/SelectionDAGNodes.h	(working copy)

+    SturtByVal        = 1<<4,  ///< Struct passed by value

This looks like a typo "sturt"


+++ utils/TableGen/CallingConvEmitter.cpp	(working copy)
@@ -67,12 +67,14 @@
      O << IndentStr << "if (";

      if (Action->isSubClassOf("CCIfType")) {
+      O << "!(ArgFlags & ISD::ParamFlags::StructByVal) && (";


I don't think CCIfType should automatically mask out byval  
arguments.  Instead, Plz add a new "notbyval" predicate or something  
so that this logic is explicit in the .td files.


Please add code to the verifier that checks that the "byval"  
attribute is only applied to pointers.  You should find similar code  
that handles "stret".

Thanks Rafael!

-Chris