Add MM register mapping from CodeView to MC register id

[LuoYuanke]

Summary

The MM register mapping from CodeView to MC register id is missing. The patch add the mapping.

[hans]

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?

[LuoYuanke]

In 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?

[hans]

In D60437#1462279, @LuoYuanke wrote:

In 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?

[LuoYuanke]

In D60437#1462379, @hans wrote:

In D60437#1462279, @LuoYuanke wrote:

In 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?

[hans]

In D60437#1462644, @LuoYuanke wrote:

In D60437#1462379, @hans wrote:

In D60437#1462279, @LuoYuanke wrote:

In 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.

[LuoYuanke]

In D60437#1462660, @hans wrote:

In D60437#1462644, @LuoYuanke wrote:

In D60437#1462379, @hans wrote:

In D60437#1462279, @LuoYuanke wrote:

In 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, @hans wrote:

In D60437#1462644, @LuoYuanke wrote:

In D60437#1462379, @hans wrote:

In D60437#1462279, @LuoYuanke wrote:

In 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);
}

[hans]

Okay, let's not worry too much about the test case, the patch seems obviously good.

Do you have commit access, or would you like me to commit for you?

[LuoYuanke]

In D60437#1462731, @hans wrote:

Okay, let's not worry too much about the test case, the patch seems obviously good.

Do you have commit access, or would you like me to commit for you?

Thank you! I just get the commit permission this week. :) I have committed the patch to llvm repo.

llvm/trunk/lib/Target/X86/MCTargetDesc/X86MCTargetDesc.cpp

MCPhysReg Reg;
       {codeview::RegisterId::ST1, X86::FP1},
       {codeview::RegisterId::ST2, X86::FP2},
       {codeview::RegisterId::ST3, X86::FP3},
       {codeview::RegisterId::ST4, X86::FP4},
       {codeview::RegisterId::ST5, X86::FP5},
       {codeview::RegisterId::ST6, X86::FP6},
       {codeview::RegisterId::ST7, X86::FP7},
 
+      {codeview::RegisterId::MM0, X86::MM0},
+      {codeview::RegisterId::MM1, X86::MM1},
+      {codeview::RegisterId::MM2, X86::MM2},
+      {codeview::RegisterId::MM3, X86::MM3},
+      {codeview::RegisterId::MM4, X86::MM4},
+      {codeview::RegisterId::MM5, X86::MM5},
+      {codeview::RegisterId::MM6, X86::MM6},
+      {codeview::RegisterId::MM7, X86::MM7},
+
       {codeview::RegisterId::XMM0, X86::XMM0},
       {codeview::RegisterId::XMM1, X86::XMM1},
       {codeview::RegisterId::XMM2, X86::XMM2},
       {codeview::RegisterId::XMM3, X86::XMM3},
       {codeview::RegisterId::XMM4, X86::XMM4},
       {codeview::RegisterId::XMM5, X86::XMM5},
       {codeview::RegisterId::XMM6, X86::XMM6},
       {codeview::RegisterId::XMM7, X86::XMM7},