[llvm] r317707 - [WebAssembly] Revise the strategy for inline asm.

Dan Gohman via llvm-commits llvm-commits at lists.llvm.org
Tue Sep 4 10:47:47 PDT 2018


Hi Chandler,

Thanks for spotting that! I was able to reproduce it by adding
-verify-machineinstrs to llc. I've now fixed it in r341389.

Dan

On Tue, Sep 4, 2018 at 7:13 AM, Chandler Carruth <chandlerc at gmail.com>
wrote:

> It's possible it didn't start with this commit, but WebAssembly assert
> fails on inline-asm MI nodes when expensive-checks are enabled. There
> likely isn't a wasm bot with these checks enabled, it'd be good to add one.
>
> The crash for me looks like:
> ******************** TEST 'LLVM :: CodeGen/WebAssembly/inline-asm.ll'
> FAILED ********************
>                                                          [110/9639]
> Script:
> --
> : 'RUN: at line 1';   /home/chandlerc/src/llvm/build/dev/bin/llc <
> /home/chandlerc/src/llvm/project/llvm/test/CodeGen/WebAssembly/inline-asm.ll
> -asm-verbose=false -disable-wasm-fallthrough-return-opt -wasm-keep-r
> egisters -no-integrated-as | /home/chandlerc/src/llvm/build/dev/bin/FileCheck
> /home/chandlerc/src/llvm/project/llvm/test/CodeGen/
> WebAssembly/inline-asm.ll
> --
> Exit Code: 2
>
> Command Output (stderr):
> --
>
> # After WebAssembly Explicit Locals
> # Machine code for function foo: NoPHIs, TracksLiveness
> Function Live Ins: $arguments
>
> bb.0.entry:
>   liveins: $arguments
>   INLINEASM &"# $0 = aaa($1)" [sideeffect] [attdialect], $0:[regdef:I32],
> 0, $1:[reguse:I32], 0, !0
>   %3:i32 = GET_LOCAL_I32 0, implicit-def $arguments
>   RETURN_I32 %3:i32, implicit-def dead $arguments
>
> # End machine code for function foo.
>
> *** Bad machine code: Instruction has operand with wrong parent set ***
> - function:    foo
> - basic block: %bb.0 entry (0x4454b98)
> - instruction: INLINEASM &"# $0 = aaa($1)" [sideeffect] [attdialect],
> $0:[regdef:I32], 0, $1:[reguse:I32], 0, !0
> Stack dump:
> 0.      Program arguments: /home/chandlerc/src/llvm/build/dev/bin/llc
> -asm-verbose=false -disable-wasm-fallthrough-return-opt
> -wasm-keep-registers -no-integrated-as
> 1.      Running pass 'Function Pass Manager' on module '<stdin>'.
> 2.      Running pass 'Verify generated machine code' on function '@foo'
> #0 0x00000000032401a4 llvm::sys::PrintStackTrace(llvm::raw_ostream&)
> /home/chandlerc/src/llvm/project/llvm/lib/Support/Unix/Signals.inc:490:13
> #1 0x0000000003240391 PrintStackTraceSignalHandler(void*)
> /home/chandlerc/src/llvm/project/llvm/lib/Support/Unix/Signals.inc:554:1
> #2 0x000000000323e4e5 llvm::sys::RunSignalHandlers()
> /home/chandlerc/src/llvm/project/llvm/lib/Support/Signals.cpp:68:18
> #3 0x00000000032404c1 SignalHandler(int) /home/chandlerc/src/llvm/
> project/llvm/lib/Support/Unix/Signals.inc:353:1
> #4 0x00007f805ea74610 __restore_rt (/lib64/libpthread.so.0+0x14610)
> #5 0x00000000029d7e11 getDesc /home/chandlerc/src/llvm/
> project/llvm/include/llvm/CodeGen/MachineInstr.h:400:48
> #6 0x00000000029d7e11 (anonymous namespace)::MachineVerifier::
> visitMachineOperand(llvm::MachineOperand const*, unsigned int)
> /home/chandlerc/src/llvm/project/llvm/lib/CodeGen/
> MachineVerifier.cpp:1097:0
> #7 0x00000000029d4328 (anonymous namespace)::MachineVerifier::verify(llvm::MachineFunction&)
> /home/chandlerc/src/llvm/project/llvm/lib/CodeGen/
> MachineVerifier.cpp:426:64
>
>
> On Wed, Nov 8, 2017 at 8:18 PM Dan Gohman via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
>> Author: djg
>> Date: Wed Nov  8 11:18:08 2017
>> New Revision: 317707
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=317707&view=rev
>> Log:
>> [WebAssembly] Revise the strategy for inline asm.
>>
>> Previously, an "r" constraint would mean the compiler provides a value
>> on WebAssembly's operand stack. This was tricky to use properly,
>> particularly since it isn't possible to declare a new local from within
>> an inline asm string.
>>
>> With this patch, "r" provides the value in a WebAssembly local, and the
>> local index is provided to the inline asm string. This requires inline
>> asm to use get_local and set_local to read the register. This does
>> potentially result in larger code size, however inline asm should
>> hopefully be quite rare in WebAssembly.
>>
>> This also means that the "m" constraint can no longer be supported, as
>> WebAssembly has nothing like a "memory operand" that includes an
>> implicit get_local.
>>
>> This fixes PR34599 for the wasm32-unknown-unknown-wasm target (though
>> not for the ELF target).
>>
>> Modified:
>>     llvm/trunk/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp
>>     llvm/trunk/lib/Target/WebAssembly/WebAssemblyExplicitLocals.cpp
>>     llvm/trunk/test/CodeGen/WebAssembly/inline-asm.ll
>>
>> Modified: llvm/trunk/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/
>> WebAssembly/WebAssemblyAsmPrinter.cpp?rev=317707&r1=317706&r2=317707&
>> view=diff
>> ============================================================
>> ==================
>> --- llvm/trunk/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp
>> (original)
>> +++ llvm/trunk/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp Wed Nov
>> 8 11:18:08 2017
>> @@ -267,12 +267,11 @@ bool WebAssemblyAsmPrinter::PrintAsmMemo
>>    if (AsmVariant != 0)
>>      report_fatal_error("There are no defined alternate asm variants");
>>
>> -  if (!ExtraCode) {
>> -    // TODO: For now, we just hard-code 0 as the constant offset; teach
>> -    // SelectInlineAsmMemoryOperand how to do address mode matching.
>> -    OS << "0(" + regToString(MI->getOperand(OpNo)) + ')';
>> -    return false;
>> -  }
>> +  // The current approach to inline asm is that "r" constraints are
>> expressed
>> +  // as local indices, rather than values on the operand stack. This
>> simplifies
>> +  // using "r" as it eliminates the need to push and pop the values in a
>> +  // particular order, however it also makes it impossible to have an "m"
>> +  // constraint. So we don't support it.
>>
>>    return AsmPrinter::PrintAsmMemoryOperand(MI, OpNo, AsmVariant,
>> ExtraCode, OS);
>>  }
>>
>> Modified: llvm/trunk/lib/Target/WebAssembly/WebAssemblyExplicitLocals.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/
>> WebAssembly/WebAssemblyExplicitLocals.cpp?rev=317707&r1=317706&r2=
>> 317707&view=diff
>> ============================================================
>> ==================
>> --- llvm/trunk/lib/Target/WebAssembly/WebAssemblyExplicitLocals.cpp
>> (original)
>> +++ llvm/trunk/lib/Target/WebAssembly/WebAssemblyExplicitLocals.cpp Wed
>> Nov  8 11:18:08 2017
>> @@ -294,6 +294,17 @@ bool WebAssemblyExplicitLocals::runOnMac
>>
>>          unsigned OldReg = MO.getReg();
>>
>> +        // Inline asm may have a def in the middle of the operands. Our
>> contract
>> +        // with inline asm register operands is to provide local indices
>> as
>> +        // immediates.
>> +        if (MO.isDef()) {
>> +          assert(MI.getOpcode() == TargetOpcode::INLINEASM);
>> +          unsigned LocalId = getLocalId(Reg2Local, CurLocal, OldReg);
>> +          MRI.removeRegOperandFromUseList(&MO);
>> +          MO = MachineOperand::CreateImm(LocalId);
>> +          continue;
>> +        }
>> +
>>          // If we see a stackified register, prepare to insert subsequent
>>          // get_locals before the start of its tree.
>>          if (MFI.isVRegStackified(OldReg)) {
>> @@ -301,6 +312,15 @@ bool WebAssemblyExplicitLocals::runOnMac
>>            continue;
>>          }
>>
>> +        // Our contract with inline asm register operands is to provide
>> local
>> +        // indices as immediates.
>> +        if (MI.getOpcode() == TargetOpcode::INLINEASM) {
>> +          unsigned LocalId = getLocalId(Reg2Local, CurLocal, OldReg);
>> +          MRI.removeRegOperandFromUseList(&MO);
>> +          MO = MachineOperand::CreateImm(LocalId);
>> +          continue;
>> +        }
>> +
>>          // Insert a get_local.
>>          unsigned LocalId = getLocalId(Reg2Local, CurLocal, OldReg);
>>          const TargetRegisterClass *RC = MRI.getRegClass(OldReg);
>>
>> Modified: llvm/trunk/test/CodeGen/WebAssembly/inline-asm.ll
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/
>> CodeGen/WebAssembly/inline-asm.ll?rev=317707&r1=317706&
>> r2=317707&view=diff
>> ============================================================
>> ==================
>> --- llvm/trunk/test/CodeGen/WebAssembly/inline-asm.ll (original)
>> +++ llvm/trunk/test/CodeGen/WebAssembly/inline-asm.ll Wed Nov  8
>> 11:18:08 2017
>> @@ -1,4 +1,4 @@
>> -; RUN: llc < %s -asm-verbose=false -disable-wasm-fallthrough-return-opt
>> -disable-wasm-explicit-locals -no-integrated-as | FileCheck %s
>> +; RUN: llc < %s -asm-verbose=false -disable-wasm-fallthrough-return-opt
>> -no-integrated-as | FileCheck %s
>>
>>  ; Test basic inline assembly. Pass -no-integrated-as since these aren't
>>  ; actually valid assembly syntax.
>> @@ -10,33 +10,24 @@ target triple = "wasm32-unknown-unknown-
>>  ; CHECK-NEXT: .param i32{{$}}
>>  ; CHECK-NEXT: .result i32{{$}}
>>  ; CHECK-NEXT: #APP{{$}}
>> -; CHECK-NEXT: # $0 = aaa($0){{$}}
>> +; CHECK-NEXT: # 0 = aaa(0){{$}}
>>  ; CHECK-NEXT: #NO_APP{{$}}
>> -; CHECK-NEXT: return $0{{$}}
>> +; CHECK-NEXT: get_local $push0=, 0{{$}}
>> +; CHECK-NEXT: return $pop0{{$}}
>>  define i32 @foo(i32 %r) {
>>  entry:
>>    %0 = tail call i32 asm sideeffect "# $0 = aaa($1)", "=r,r"(i32 %r) #0,
>> !srcloc !0
>>    ret i32 %0
>>  }
>>
>> -; CHECK-LABEL: bar:
>> -; CHECK-NEXT: .param i32, i32{{$}}
>> -; CHECK-NEXT: #APP{{$}}
>> -; CHECK-NEXT: # 0($1) = bbb(0($0)){{$}}
>> -; CHECK-NEXT: #NO_APP{{$}}
>> -; CHECK-NEXT: return{{$}}
>> -define void @bar(i32* %r, i32* %s) {
>> -entry:
>> -  tail call void asm sideeffect "# $0 = bbb($1)", "=*m,*m"(i32* %s, i32*
>> %r) #0, !srcloc !1
>> -  ret void
>> -}
>> -
>>  ; CHECK-LABEL: imm:
>>  ; CHECK-NEXT: .result i32{{$}}
>> +; CHECK-NEXT: .local i32{{$}}
>>  ; CHECK-NEXT: #APP{{$}}
>> -; CHECK-NEXT: # $0 = ccc(42){{$}}
>> +; CHECK-NEXT: # 0 = ccc(42){{$}}
>>  ; CHECK-NEXT: #NO_APP{{$}}
>> -; CHECK-NEXT: return $0{{$}}
>> +; CHECK-NEXT: get_local $push0=, 0{{$}}
>> +; CHECK-NEXT: return $pop0{{$}}
>>  define i32 @imm() {
>>  entry:
>>    %0 = tail call i32 asm sideeffect "# $0 = ccc($1)", "=r,i"(i32 42) #0,
>> !srcloc !2
>> @@ -47,9 +38,10 @@ entry:
>>  ; CHECK-NEXT: .param i64{{$}}
>>  ; CHECK-NEXT: .result i64{{$}}
>>  ; CHECK-NEXT: #APP{{$}}
>> -; CHECK-NEXT: # $0 = aaa($0){{$}}
>> +; CHECK-NEXT: # 0 = aaa(0){{$}}
>>  ; CHECK-NEXT: #NO_APP{{$}}
>> -; CHECK-NEXT: return $0{{$}}
>> +; CHECK-NEXT: get_local $push0=, 0{{$}}
>> +; CHECK-NEXT: return $pop0{{$}}
>>  define i64 @foo_i64(i64 %r) {
>>  entry:
>>    %0 = tail call i64 asm sideeffect "# $0 = aaa($1)", "=r,r"(i64 %r) #0,
>> !srcloc !0
>> @@ -57,16 +49,20 @@ entry:
>>  }
>>
>>  ; CHECK-LABEL: X_i16:
>> -; CHECK: foo $1{{$}}
>> -; CHECK: i32.store16 0($0), $1{{$}}
>> +; CHECK: foo 1{{$}}
>> +; CHECK: get_local $push[[S0:[0-9]+]]=, 0{{$}}
>> +; CHECK-NEXT: get_local $push[[S1:[0-9]+]]=, 1{{$}}
>> +; CHECK-NEXT: i32.store16 0($pop[[S0]]), $pop[[S1]]{{$}}
>>  define void @X_i16(i16 * %t) {
>>    call void asm sideeffect "foo $0", "=*X,~{dirflag},~{fpsr},~{flags},~{memory}"(i16*
>> %t)
>>    ret void
>>  }
>>
>>  ; CHECK-LABEL: X_ptr:
>> -; CHECK: foo $1{{$}}
>> -; CHECK: i32.store 0($0), $1{{$}}
>> +; CHECK: foo 1{{$}}
>> +; CHECK: get_local $push[[S0:[0-9]+]]=, 0{{$}}
>> +; CHECK-NEXT: get_local $push[[S1:[0-9]+]]=, 1{{$}}
>> +; CHECK-NEXT: i32.store 0($pop[[S0]]), $pop[[S1]]{{$}}
>>  define void @X_ptr(i16 ** %t) {
>>    call void asm sideeffect "foo $0", "=*X,~{dirflag},~{fpsr},~{flags},~{memory}"(i16**
>> %t)
>>    ret void
>> @@ -87,6 +83,20 @@ define void @varname() {
>>    ret void
>>  }
>>
>> +; CHECK-LABEL: r_constraint
>> +; CHECK:      i32.const $push[[S0:[0-9]+]]=, 0{{$}}
>> +; CHECK-NEXT: set_local [[L0:[0-9]+]], $pop[[S0]]{{$}}
>> +; CHECK-NEXT: i32.const $push[[S1:[0-9]+]]=, 37{{$}}
>> +; CHECK-NEXT: set_local [[L1:[0-9]+]], $pop[[S1]]{{$}}
>> +; CHECK:      foo [[L2:[0-9]+]], 1, [[L0]], [[L1]]{{$}}
>> +; CHECK:      get_local $push{{[0-9]+}}=, [[L2]]{{$}}
>> +define hidden i32 @r_constraint(i32 %a, i32 %y) {
>> +entry:
>> +  %z = bitcast i32 0 to i32
>> +  %t0 = tail call i32 asm "foo $0, $1, $2, $3", "=r,r,r,r"(i32 %y, i32
>> %z, i32 37) #0, !srcloc !0
>> +  ret i32 %t0
>> +}
>> +
>>  attributes #0 = { nounwind }
>>
>>  !0 = !{i32 47}
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180904/94abf8d0/attachment.html>


More information about the llvm-commits mailing list