From 4ddceef928720fda2db603a2b6501838f6047c86 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Fri, 27 Aug 2021 22:18:07 +0200 Subject: [PATCH] [WebAssembly] Fix FastISel of condition in different block (PR51651) If the icmp is in a different block, then the register for the icmp operand may not be initialized, as it nominally does not have cross-block uses. Add a check that the icmp is in the same block as the branch, which should be the common case. This matches what X86 FastISel does: https://github.com/llvm/llvm-project/blob/5b6b090cf2129228f05d7d0f504676b67f7524cf/llvm/lib/Target/X86/X86FastISel.cpp#L1648 The "not" transform that could have a similar issue is dropped entirely, because it is currently dead: The incoming value is a branch or select condition of type i1, but this code requires an i32 to trigger. Fixes https://bugs.llvm.org/show_bug.cgi?id=51651. Differential Revision: https://reviews.llvm.org/D108840 (cherry picked from commit 16086d47c0d0cd08ffae8e69a69c88653e654d01) --- .../WebAssembly/WebAssemblyFastISel.cpp | 20 +++++----- test/CodeGen/WebAssembly/pr51651.ll | 39 +++++++++++++++++++ 2 files changed, 48 insertions(+), 11 deletions(-) create mode 100644 test/CodeGen/WebAssembly/pr51651.ll diff --git a/lib/Target/WebAssembly/WebAssemblyFastISel.cpp b/lib/Target/WebAssembly/WebAssemblyFastISel.cpp index 171d59ae4c6..ae5108b0cb0 100644 --- a/lib/Target/WebAssembly/WebAssemblyFastISel.cpp +++ b/lib/Target/WebAssembly/WebAssemblyFastISel.cpp @@ -157,7 +157,7 @@ private: void addLoadStoreOperands(const Address &Addr, const MachineInstrBuilder &MIB, MachineMemOperand *MMO); unsigned maskI1Value(unsigned Reg, const Value *V); - unsigned getRegForI1Value(const Value *V, bool &Not); + unsigned getRegForI1Value(const Value *V, const BasicBlock *BB, bool &Not); unsigned zeroExtendToI32(unsigned Reg, const Value *V, MVT::SimpleValueType From); unsigned signExtendToI32(unsigned Reg, const Value *V, @@ -418,20 +418,17 @@ unsigned WebAssemblyFastISel::maskI1Value(unsigned Reg, const Value *V) { return zeroExtendToI32(Reg, V, MVT::i1); } -unsigned WebAssemblyFastISel::getRegForI1Value(const Value *V, bool &Not) { +unsigned WebAssemblyFastISel::getRegForI1Value(const Value *V, + const BasicBlock *BB, + bool &Not) { if (const auto *ICmp = dyn_cast(V)) if (const ConstantInt *C = dyn_cast(ICmp->getOperand(1))) - if (ICmp->isEquality() && C->isZero() && C->getType()->isIntegerTy(32)) { + if (ICmp->isEquality() && C->isZero() && C->getType()->isIntegerTy(32) && + ICmp->getParent() == BB) { Not = ICmp->isTrueWhenEqual(); return getRegForValue(ICmp->getOperand(0)); } - Value *NotV; - if (match(V, m_Not(m_Value(NotV))) && V->getType()->isIntegerTy(32)) { - Not = true; - return getRegForValue(NotV); - } - Not = false; unsigned Reg = getRegForValue(V); if (Reg == 0) @@ -912,7 +909,8 @@ bool WebAssemblyFastISel::selectSelect(const Instruction *I) { const auto *Select = cast(I); bool Not; - unsigned CondReg = getRegForI1Value(Select->getCondition(), Not); + unsigned CondReg = + getRegForI1Value(Select->getCondition(), I->getParent(), Not); if (CondReg == 0) return false; @@ -1312,7 +1310,7 @@ bool WebAssemblyFastISel::selectBr(const Instruction *I) { MachineBasicBlock *FBB = FuncInfo.MBBMap[Br->getSuccessor(1)]; bool Not; - unsigned CondReg = getRegForI1Value(Br->getCondition(), Not); + unsigned CondReg = getRegForI1Value(Br->getCondition(), Br->getParent(), Not); if (CondReg == 0) return false; diff --git a/test/CodeGen/WebAssembly/pr51651.ll b/test/CodeGen/WebAssembly/pr51651.ll new file mode 100644 index 00000000000..70ddcf07dc8 --- /dev/null +++ b/test/CodeGen/WebAssembly/pr51651.ll @@ -0,0 +1,39 @@ +; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py +; RUN: llc -O0 -mtriple=wasm32-unknown-unknown -wasm-disable-explicit-locals -wasm-keep-registers < %s | FileCheck %s + +define i32 @test(i8* %p, i8* %p2) { +; CHECK-LABEL: test: +; CHECK: .functype test (i32, i32) -> (i32) +; CHECK-NEXT: # %bb.0: +; CHECK-NEXT: i32.load8_u $3=, 0($0) +; CHECK-NEXT: i32.eqz $2=, $3 +; CHECK-NEXT: i32.store8 0($1), $3 +; CHECK-NEXT: # %bb.1: # %bb2 +; CHECK-NEXT: i32.const $4=, 1 +; CHECK-NEXT: i32.and $5=, $2, $4 +; CHECK-NEXT: block +; CHECK-NEXT: br_if 0, $5 # 0: down to label0 +; CHECK-NEXT: # %bb.2: # %bb4 +; CHECK-NEXT: i32.const $6=, 0 +; CHECK-NEXT: return $6 +; CHECK-NEXT: .LBB0_3: # %bb3 +; CHECK-NEXT: end_block # label0: +; CHECK-NEXT: i32.const $7=, 1 +; CHECK-NEXT: return $7 + %v = load i8, i8* %p + %v.ext = zext i8 %v to i32 + %cond = icmp eq i32 %v.ext, 0 + ; Cause FastISel abort. + %shl = shl i8 %v, 0 + store i8 %shl, i8* %p2 + br label %bb2 + +bb2: + br i1 %cond, label %bb3, label %bb4 + +bb4: + ret i32 0 + +bb3: + ret i32 1 +}