llvm dev - Jun 2010 - [LLVMdev] r98938 broke argument passing on MSP430?

Hello,

Yesterday I noticed that MSP430 argument passing is broken in trunk;
see http://llvm.org/PR6573 for details and testcases.  The problem is
that calls aren't being preceded by instructions that put the
arguments into registers.  I backtracked my working copy and then
stepped forward until it broke between r98937 and r98938.  Refining
further, I found that rolling back the single-statement change to
utils/TableGen/DAGISelMatcherGen.cpp -- something about variadic
instructions that was probably OK for x86, but MSP430 doesn't have
variadic instructions -- restored the correct behavior.  Seems like
r98938 either introduced a bug in DAGISelMatcherGen.cpp or exposed a
bug in MSP430InstrInfo.td (or similar).  That's Chris's commit; could
he or someone else who's comfortable with LLVM internals (I'm not)
help me root out the problem?

I'm asking here because I don't know if this change broke other
non-x86 architectures.

-ben

On Fri, Jun 11, 2010 at 2:55 PM, Ben Ransford <ransford at cs.umass.edu>
wrote:
> Yesterday I noticed that MSP430 argument passing is broken in trunk;
> see http://llvm.org/PR6573 for details and testcases.  The problem is
> that calls aren't being preceded by instructions that put the
> arguments into registers.  I backtracked my working copy and then
> stepped forward until it broke between r98937 and r98938.  Refining
> further, I found that rolling back the single-statement change to
> utils/TableGen/DAGISelMatcherGen.cpp -- something about variadic
> instructions that was probably OK for x86, but MSP430 doesn't have
> variadic instructions -- restored the correct behavior.  Seems like
> r98938 either introduced a bug in DAGISelMatcherGen.cpp or exposed a
> bug in MSP430InstrInfo.td (or similar).  That's Chris's commit;
could
> he or someone else who's comfortable with LLVM internals (I'm not)
> help me root out the problem?
Anton graciously helped me out of band.  I'm attaching his patch
(recomputed against ToT), which fixes a bug in MSP430InstrInfo.td.
I'm afraid I don't know how to write a testcase for this bug.

(Key insight from Anton: attaching "SDNPVariadic" to
"MSP430call"
indicates that the *instruction selector pattern*, not the call
instruction, is variadic.)

-ben
-------------- next part --------------
A non-text attachment was scrubbed...
Name: msp430-call-variadic.patch
Type: application/octet-stream
Size: 677 bytes
Desc: not available
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20100621/f2189cd5/attachment.obj>

On Jun 21, 2010, at 1:32 PM, Ben Ransford wrote:
> On Fri, Jun 11, 2010 at 2:55 PM, Ben Ransford <ransford at
cs.umass.edu> wrote:
>> Yesterday I noticed that MSP430 argument passing is broken in trunk;
>> see http://llvm.org/PR6573 for details and testcases.  The problem is
>> that calls aren't being preceded by instructions that put the
>> arguments into registers.  I backtracked my working copy and then
>> stepped forward until it broke between r98937 and r98938.  Refining
>> further, I found that rolling back the single-statement change to
>> utils/TableGen/DAGISelMatcherGen.cpp -- something about variadic
>> instructions that was probably OK for x86, but MSP430 doesn't have
>> variadic instructions -- restored the correct behavior.  Seems like
>> r98938 either introduced a bug in DAGISelMatcherGen.cpp or exposed a
>> bug in MSP430InstrInfo.td (or similar).  That's Chris's commit;
could
>> he or someone else who's comfortable with LLVM internals (I'm
not)
>> help me root out the problem?
> 
> Anton graciously helped me out of band.  I'm attaching his patch
> (recomputed against ToT), which fixes a bug in MSP430InstrInfo.td.
> I'm afraid I don't know how to write a testcase for this bug.
> 
> (Key insight from Anton: attaching "SDNPVariadic" to
"MSP430call"
> indicates that the *instruction selector pattern*, not the call
> instruction, is variadic.)
Applied in r106722, thanks!

-Chris