[PATCH] D60437: Add MM register mapping from CodeView to MC register id

Thu Apr 11 07:12:30 PDT 2019

LuoYuanke added a comment.

In D60437#1462660 <https://reviews.llvm.org/D60437#1462660>, @hans wrote:

> In D60437#1462644 <https://reviews.llvm.org/D60437#1462644>, @LuoYuanke wrote:
>
> > In D60437#1462379 <https://reviews.llvm.org/D60437#1462379>, @hans wrote:
> >
> > > In D60437#1462279 <https://reviews.llvm.org/D60437#1462279>, @LuoYuanke wrote:
> > >
> > > > In D60437#1461210 <https://reviews.llvm.org/D60437#1461210>, @hans wrote:
> > > >
> > > > > I'm not familiar with the CodeView stuff, rnk is the better person for that, but would it be possible to add a test that exercises this mapping? Or to put it another way, what does this fix that's currently broken, and is it possible to add a test for it?
> > > >
> > > >
> > > > I try to add such test case, but I don't find any existing test case for mapping codeveiw register to MC register.
> > > >  @rnk
> > > >  Do you know if there is any reference test case in llvm/test code?
> > >
> > >
> > > You can try removing a bunch of the entries in RegMap and see which tests fail. They're all in llvm/tests/DebugInfo/COFF/
> > >  I don't think there's any test for the mapping itself, but the tests rely on it to work.
> > >
> > > That's what my second question was about: what is currently not working that your patch is fixing, and can that be used to write a test?
> >
> >
> > Thank you for the suggestion. I disable the code of xmm0 - xmm7 mapping and there is no case failure in llvm/tests/DebugInfo/COFF/. However when I disable general register (rax -r15), some of the test cases (.e.g test/DebugInfo/COFF/types-basic.ll) fail. There is pretty much code for the failed test case which I'm investigating. To create a small test case, we may need to cause much pressure on register allocator for mm0 - mm7 with __m64 variable in c code. However I have not figured out how to created such case.
> >  Nevertheless, does anybody know why the mm0-mm7 registers are not mapped. Is there any reasons on it?
>
>
> I'm guessing it's just an oversight.
>
> Would it be possible to use inline assembly to trigger use of these registers in some way?
>
> If this turns out to be hard, I think we can just check in your patch as is, it seems like an obvious fix.

In D60437#1462660 <https://reviews.llvm.org/D60437#1462660>, @hans wrote:

> In D60437#1462644 <https://reviews.llvm.org/D60437#1462644>, @LuoYuanke wrote:
>
> > In D60437#1462379 <https://reviews.llvm.org/D60437#1462379>, @hans wrote:
> >
> > > In D60437#1462279 <https://reviews.llvm.org/D60437#1462279>, @LuoYuanke wrote:
> > >
> > > > In D60437#1461210 <https://reviews.llvm.org/D60437#1461210>, @hans wrote:
> > > >
> > > > > I'm not familiar with the CodeView stuff, rnk is the better person for that, but would it be possible to add a test that exercises this mapping? Or to put it another way, what does this fix that's currently broken, and is it possible to add a test for it?
> > > >
> > > >
> > > > I try to add such test case, but I don't find any existing test case for mapping codeveiw register to MC register.
> > > >  @rnk
> > > >  Do you know if there is any reference test case in llvm/test code?
> > >
> > >
> > > You can try removing a bunch of the entries in RegMap and see which tests fail. They're all in llvm/tests/DebugInfo/COFF/
> > >  I don't think there's any test for the mapping itself, but the tests rely on it to work.
> > >
> > > That's what my second question was about: what is currently not working that your patch is fixing, and can that be used to write a test?
> >
> >
> > Thank you for the suggestion. I disable the code of xmm0 - xmm7 mapping and there is no case failure in llvm/tests/DebugInfo/COFF/. However when I disable general register (rax -r15), some of the test cases (.e.g test/DebugInfo/COFF/types-basic.ll) fail. There is pretty much code for the failed test case which I'm investigating. To create a small test case, we may need to cause much pressure on register allocator for mm0 - mm7 with __m64 variable in c code. However I have not figured out how to created such case.
> >  Nevertheless, does anybody know why the mm0-mm7 registers are not mapped. Is there any reasons on it?
>
>
> I'm guessing it's just an oversight.
>
> Would it be possible to use inline assembly to trigger use of these registers in some way?
>
> If this turns out to be hard, I think we can just check in your patch as is, it seems like an obvious fix.

I try to write the test case as below. But it seems mmx register can not be specified explicitly in the clobber list or input list. If I specify "y" constrain, then compiler always allocate mm0 and mm1 register. If I specify the mmx register in the assembly code, then compiler is not aware of it on register allocation.

  void test_mmx() {
    __m64 t1, t2, t3, t4;
    //asm("paddsw %0, %1"::"mm1"(t1), "mm2"(t2):);
    asm("paddsw %%mm0, %%mm1":::);
    asm("paddsw %%mm1, %%mm2":::);
    asm("paddsw %%mm2, %%mm3":::);
    asm("paddsw %%mm3, %%mm4":::);
    asm("paddsw %%mm4, %%mm5":::);
    asm("paddsw %%mm5, %%mm6":::);
    asm("paddsw %0, %1"::"y"(t3), "y"(t4):);
    asm("paddsw %0, %1"::"y"(t1), "y"(t2):);
    asm("paddsw %0, %1"::"y"(t1), "y"(t3):);
    __m64 t5 = _m_paddsw(t1, t2);
    __m64 t6 = _m_psubusb(t3, t4);
    __m64 t7 = _m_paddsw(t5, t6);
  }

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60437/new/

https://reviews.llvm.org/D60437