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