On Feb 15, 2010, at 1:04 PM, David Greene wrote:>> FWIW, this is because you broke the encoding of an instruction in your >> patch. This is incorrect: >> >> +def MOVNTDQ_64mr : PSI<0xE7, MRMDestMem, (outs), (ins f128mem:$dst, >> VR128:$src), + "movntdq\t{$src, $dst|$dst, $src}", >> + [(alignednontemporalstore (v2f64 VR128:$src), >> addr:$dst)]>; >> >> Please don't check in patches when you know that they break testcases. > > I certainly didn't know it broke anything. I even checked the buildbot and it > looked like it failed before I ever applied anything. Apologies for the mix- > up.FYI, if you want to check the encoding of an instruction, you can do so with a command like this: $ cat t.s movntdq %xmm5, 3735928559(%ebx,%ecx,8) $ llvm-mc t.s -show-encoding -show-inst .section __TEXT,__text,regular,pure_instructions movntdq %xmm5, 3735928559(%ebx,%ecx,8) ## encoding: [0x66,0x0f,0xe7,0xac,0xcb,0xef,0xbe,0xad,0xde] ## <MCInst #1358 MOVNTDQmr ## <MCOperand Reg:29> ## <MCOperand Imm:8> ## <MCOperand Reg:38> ## <MCOperand Imm:3735928559> ## <MCOperand Reg:0> ## <MCOperand Reg:138>> This shows that the 'MOVNTDQmr' instruction is being used along with the encoding. -Chris
On Monday 15 February 2010 15:08:22 Chris Lattner wrote:> On Feb 15, 2010, at 1:04 PM, David Greene wrote: > >> FWIW, this is because you broke the encoding of an instruction in your > >> patch. This is incorrect: > >> > >> +def MOVNTDQ_64mr : PSI<0xE7, MRMDestMem, (outs), (ins f128mem:$dst, > >> VR128:$src), + "movntdq\t{$src, $dst|$dst, $src}", > >> + [(alignednontemporalstore (v2f64 VR128:$src), > >> addr:$dst)]>; > >> > >> Please don't check in patches when you know that they break testcases. > > > > I certainly didn't know it broke anything. I even checked the buildbot > > and it looked like it failed before I ever applied anything. Apologies > > for the mix- up. > > FYI, if you want to check the encoding of an instruction, you can do so > with a command like this:Thanks. It's strange that the buidbot shows this test failing long before I made the change. Why would that be? In any event, I've got the problem fixed. It was a cut-n-paste oversight on my part. As for documentation, where should that go in LangRef.html? There's nothing there yet that really talks about special metadata. I can make up a section if that seems appropriate. -Dave
On Feb 15, 2010, at 1:17 PM, David Greene wrote:> On Monday 15 February 2010 15:08:22 Chris Lattner wrote: >> On Feb 15, 2010, at 1:04 PM, David Greene wrote: >>>> FWIW, this is because you broke the encoding of an instruction in your >>>> patch. This is incorrect: >>>> >>>> +def MOVNTDQ_64mr : PSI<0xE7, MRMDestMem, (outs), (ins f128mem:$dst, >>>> VR128:$src), + "movntdq\t{$src, $dst|$dst, $src}", >>>> + [(alignednontemporalstore (v2f64 VR128:$src), >>>> addr:$dst)]>; >>>> >>>> Please don't check in patches when you know that they break testcases. >>> >>> I certainly didn't know it broke anything. I even checked the buildbot >>> and it looked like it failed before I ever applied anything. Apologies >>> for the mix- up. >> >> FYI, if you want to check the encoding of an instruction, you can do so >> with a command like this: > > Thanks. It's strange that the buidbot shows this test failing long before I > made the change. Why would that be?It didn't. Here's a good build: http://smooshlab.apple.com:8010/builders/clang-i386-darwin9/builds/5019 The following two builds are broken by your previous bug: http://smooshlab.apple.com:8010/builders/clang-i386-darwin9/builds/5020 http://smooshlab.apple.com:8010/builders/clang-i386-darwin9/builds/5021 with that fixed, the subsequent builds failed these regression tests: http://smooshlab.apple.com:8010/builders/clang-i386-darwin9/builds/5022> As for documentation, where should that go in LangRef.html? There's > nothing there yet that really talks about special metadata. I can make up > a section if that seems appropriate.Add it to the #i_store section, thanks. -Chris
Unless others find this objectionable, it would be nice to see the grammar for metadata instruction attachment in the http://llvm.org/docs/LangRef.html#metadata section. The grammar you gave earlier seems to work. Garrison On Feb 15, 2010, at 16:17, David Greene wrote:> On Monday 15 February 2010 15:08:22 Chris Lattner wrote: >> On Feb 15, 2010, at 1:04 PM, David Greene wrote: >>>> FWIW, this is because you broke the encoding of an instruction in your >>>> patch. This is incorrect: >>>> >>>> +def MOVNTDQ_64mr : PSI<0xE7, MRMDestMem, (outs), (ins f128mem:$dst, >>>> VR128:$src), + "movntdq\t{$src, $dst|$dst, $src}", >>>> + [(alignednontemporalstore (v2f64 VR128:$src), >>>> addr:$dst)]>; >>>> >>>> Please don't check in patches when you know that they break testcases. >>> >>> I certainly didn't know it broke anything. I even checked the buildbot >>> and it looked like it failed before I ever applied anything. Apologies >>> for the mix- up. >> >> FYI, if you want to check the encoding of an instruction, you can do so >> with a command like this: > > Thanks. It's strange that the buidbot shows this test failing long before I > made the change. Why would that be? > > In any event, I've got the problem fixed. It was a cut-n-paste oversight on > my part. > > As for documentation, where should that go in LangRef.html? There's > nothing there yet that really talks about special metadata. I can make up > a section if that seems appropriate. > > -Dave > > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev