This is an archive of the discontinued LLVM Phabricator instance.

[Microsoft][C++] Clang doesn't support a use of "this" pointer inside inline asm
ClosedPublic

Authored by m_zuckerman on Dec 1 2015, 8:38 AM.

Details

Summary

Clang doesn’t support a use of “this” pointer inside inline asm.
When I tried to compile a class or a struct (see example) with an inline asm that contains "this" pointer.
Clang returns with an error.
This patch fixes that.

error: expected unqualified-id
For example:

struct A {
  void f() {
    __asm mov eax, this
    // error: expected unqualified-id
  }
};

Diff Detail

Event Timeline

m_zuckerman updated this revision to Diff 41519.Dec 1 2015, 8:38 AM
m_zuckerman retitled this revision from to [Microsoft][C++] Clang doesn't support a use of "this" pointer inside inline asm .
m_zuckerman updated this object.
m_zuckerman added a subscriber: llvm-commits.
rnk requested changes to this revision.Dec 1 2015, 9:38 AM
rnk edited edge metadata.
rnk added inline comments.
lib/Parse/ParseExprCXX.cpp
2567–2568

Surely this breaks the contract of ParseUnqualifiedId, which should not return success if called on a 'this' token.

test/CodeGen/ms_this.cpp
25

I think FileCheck will insist that this is spelled without a space before the colon.

It would also be better to test that %this.addr is used as an argument to an asm blob, something like this:

// CHECK: call void asm sideeffect {{.*}}%this.addr
This revision now requires changes to proceed.Dec 1 2015, 9:38 AM
m_zuckerman updated this revision to Diff 41612.Dec 2 2015, 5:21 AM
m_zuckerman edited edge metadata.
m_zuckerman marked an inline comment as done.
m_zuckerman marked an inline comment as done.Dec 7 2015, 3:27 AM

ping

rnk added inline comments.Dec 7 2015, 11:24 AM
lib/Parse/ParseExprCXX.cpp
2567–2569

I think you want to teach ParseMSAsmIdentifier to check for kw_this before calling ParseUnqualifiedId. That would make it consistent with the primary-expression production in the comments above ParseCastExpression.

m_zuckerman edited edge metadata.
rnk accepted this revision.Dec 14 2015, 2:53 PM
rnk edited edge metadata.

lgtm

test/CodeGen/ms_this.cpp
41

Is this correct? Shouldn't "this" be $1 or something?

This revision is now accepted and ready to land.Dec 14 2015, 2:53 PM
This revision was automatically updated to reflect the committed changes.
m_zuckerman added inline comments.Dec 15 2015, 6:16 AM
test/CodeGen/ms_this.cpp
41

This is depend on which mode we use 32 or 64. I changed it to be consistent with other class .