On Feb 15, 2010, at 12:53 PM, Chris Lattner wrote:> > On Feb 15, 2010, at 10:00 AM, David Greene wrote: > >> On Monday 15 February 2010 11:54:25 Óscar Fuentes wrote: >>> David Greene <dag at cray.com> writes: >>>> Sorry, I botched a commit and broke the build. I've just committed a >>>> fix. >>>> >>>> So expect to see some buildbot churning. >>> >>> Don't hurry. A buildbot already decided that I am the only culprit of >>> the breakage. :-/ >> >> Hmm...given that MC/AsmParser/X86/x86_32-encoding.s has been failing >> for a while, maybe someone should mark it XFAIL? > > It passed for me until you applied your patch, I reverted it in r96265.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. -Chris
On Monday 15 February 2010 14:59:19 Chris Lattner wrote:> On Feb 15, 2010, at 12:53 PM, Chris Lattner wrote: > > On Feb 15, 2010, at 10:00 AM, David Greene wrote: > >> On Monday 15 February 2010 11:54:25 Óscar Fuentes wrote: > >>> David Greene <dag at cray.com> writes: > >>>> Sorry, I botched a commit and broke the build. I've just committed a > >>>> fix. > >>>> > >>>> So expect to see some buildbot churning. > >>> > >>> Don't hurry. A buildbot already decided that I am the only culprit of > >>> the breakage. :-/ > >> > >> Hmm...given that MC/AsmParser/X86/x86_32-encoding.s has been failing > >> for a while, maybe someone should mark it XFAIL? > > > > It passed for me until you applied your patch, I reverted it in r96265. > > 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. -Dave
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