[PATCH] D10476: Additional fix for PR14269: Clang crashes when a bit field is used as inline assembler input / output with memory constraint

[Andrey Bokhanko]

andreybokhanko added a comment.

Hi Eric,

Thanks for looking into the patch!

In http://reviews.llvm.org/D10476#200564, @echristo wrote:

> This seems to be a bit overconstrained versus what gcc accepts on the testcase:


There are three cases that GCC accepts and clang (with my fix) doesn't:

1. Output to a bit-field:

  typedef struct test16_foo {
    unsigned int field1 : 1;
    unsigned int field2 : 2;
    unsigned int field3 : 3;
  } test16_foo;
  
  void test16()
  {
    test16_foo a;
  
    __asm__("movl $5, %0"
            : "=rm" (a.field2)); // expected-error {{reference to a non-addressable value in asm output with a memory constraint '=rm'}}
  }

GCC accepts this, but generates incorrect code, that is not even accepted by assembler:

  $ gcc test1.c
  test1.c: Assembler messages:
  test1.c:11: Error: `%al' not allowed with `movl'



2. Vector elements:

  typedef __attribute__((vector_size(16))) int test16_bar;
  
  int main()
  {
    test16_bar b = {1, 2, 3, 4};
  
    __asm__("movl $5, %0"
            : "=rm" (b[2]));
  
    return b[2];
  }

The problem here is that LLVM IR represents vectors with a specific vector type; you can't get address of a random element inside vector. Specific instructions should be used to get individual vector elements ("extractelement" and "insertelement"), but then again -- they don't provide //addresses// of elements. GCC simply treats a vector as an array of elements and computes desired address. In theory, this can be done in LLVM IR as well, but I don't think this is the right approach -- we generally can't make any assumptions on how vectors are represented by a target CPU.

Do you agree?

3. Global register variables:

  register int test16_baz asm("rbx");
  
  void test16()
  {
    __asm__("movl $5, %0"
            : "=rm" (test16_baz)); // expected-error {{reference to a non-addressable value in asm output with a memory constraint '=rm'}}
  }

The constraint here says "register *or* memory". GCC chooses register and compiles the test fine. Clang always chooses memory -- due to the following check at CGStmt.cpp:1874:

  // If this is a register output, then make the inline asm return it
  // by-value.  If this is a memory result, return the value by-reference.
  if (!Info.allowsMemory() && hasScalarEvaluationKind(OutExpr->getType())) {

I have no idea why it checks for "!info.allowsMemory()" and not for "info.allowsRegister", but this is a separate issue, not related to my patch.

When a test is re-written in a way that allows *only* memory, GCC complains as well:

  register int test16_baz asm("rbx");
  
  void test16()
  {
    __asm__("movl $5, %0"
            : "=m" (test16_baz)); // expected-error {{reference to a non-addressable value in asm output with a memory constraint '=rm'}}
  }



  $ gcc test3.c
  test3.c: In function ?test16?:
  test3.c:6:11: error: address of global register variable ?test16_baz? requested
             : "=m" (test16_baz)); // expected-error {{reference to a non-addressable value in asm output with a memory constraint '=rm'}}
             ^
  test3.c:5:3: error: invalid lvalue in asm output 0
     __asm__("movl $5, %0"
     ^



In http://reviews.llvm.org/D10476#200564, @echristo wrote:

> also, the precision in error messages is nice. :)


Done!

Please re-review.

Andrey


http://reviews.llvm.org/D10476