Mark setjmp as "returns twice" and turn off SimpleCoalescing when called.
BUG=none
R=stichnot@chromium.org
Review URL: https://codereview.chromium.org/567553003
diff --git a/src/IceIntrinsics.cpp b/src/IceIntrinsics.cpp
index 5b6ceac..e941ff9 100644
--- a/src/IceIntrinsics.cpp
+++ b/src/IceIntrinsics.cpp
@@ -24,39 +24,49 @@
namespace {
+void __attribute__((unused)) xIntrinsicInfoSizeCheck() {
+ STATIC_ASSERT(sizeof(Intrinsics::IntrinsicInfo) == 4);
+}
+
+#define INTRIN(ID, SE, RT) { Intrinsics::ID, Intrinsics::SE, Intrinsics::RT }
+
+// Build list of intrinsics with their attributes and expected prototypes.
+// List is sorted alphabetically.
const struct IceIntrinsicsEntry_ {
Intrinsics::FullIntrinsicInfo Info;
const char *IntrinsicName;
} IceIntrinsicsTable[] = {
+
#define AtomicCmpxchgInit(Overload, NameSuffix) \
{ \
{ \
- { Intrinsics::AtomicCmpxchg, true }, \
+ INTRIN(AtomicCmpxchg, SideEffects_T, ReturnsTwice_F), \
{ Overload, IceType_i32, Overload, Overload, IceType_i32, IceType_i32 }, \
- 6 \
- } \
- , "nacl.atomic.cmpxchg." NameSuffix \
+ 6 }, \
+ "nacl.atomic.cmpxchg." NameSuffix \
}
AtomicCmpxchgInit(IceType_i8, "i8"),
AtomicCmpxchgInit(IceType_i16, "i16"),
AtomicCmpxchgInit(IceType_i32, "i32"),
AtomicCmpxchgInit(IceType_i64, "i64"),
#undef AtomicCmpxchgInit
- { { { Intrinsics::AtomicFence, true }, { IceType_void, IceType_i32 }, 2 },
+
+ { { INTRIN(AtomicFence, SideEffects_T, ReturnsTwice_F),
+ { IceType_void, IceType_i32 }, 2 },
"nacl.atomic.fence" },
- { { { Intrinsics::AtomicFenceAll, true }, { IceType_void }, 1 },
+ { { INTRIN(AtomicFenceAll, SideEffects_T, ReturnsTwice_F),
+ { IceType_void }, 1 },
"nacl.atomic.fence.all" },
- { { { Intrinsics::AtomicIsLockFree, false },
+ { { INTRIN(AtomicIsLockFree, SideEffects_F, ReturnsTwice_F),
{ IceType_i1, IceType_i32, IceType_i32 }, 3 },
"nacl.atomic.is.lock.free" },
#define AtomicLoadInit(Overload, NameSuffix) \
{ \
{ \
- { Intrinsics::AtomicLoad, true } \
- , { Overload, IceType_i32, IceType_i32 }, 3 \
- } \
- , "nacl.atomic.load." NameSuffix \
+ INTRIN(AtomicLoad, SideEffects_T, ReturnsTwice_F), \
+ { Overload, IceType_i32, IceType_i32 }, 3 }, \
+ "nacl.atomic.load." NameSuffix \
}
AtomicLoadInit(IceType_i8, "i8"),
AtomicLoadInit(IceType_i16, "i16"),
@@ -67,7 +77,7 @@
#define AtomicRMWInit(Overload, NameSuffix) \
{ \
{ \
- { Intrinsics::AtomicRMW, true } \
+ INTRIN(AtomicRMW, SideEffects_T, ReturnsTwice_F) \
, { Overload, IceType_i32, IceType_i32, Overload, IceType_i32 }, 5 \
} \
, "nacl.atomic.rmw." NameSuffix \
@@ -81,7 +91,7 @@
#define AtomicStoreInit(Overload, NameSuffix) \
{ \
{ \
- { Intrinsics::AtomicStore, true } \
+ INTRIN(AtomicStore, SideEffects_T, ReturnsTwice_F) \
, { IceType_void, Overload, IceType_i32, IceType_i32 }, 4 \
} \
, "nacl.atomic.store." NameSuffix \
@@ -95,7 +105,7 @@
#define BswapInit(Overload, NameSuffix) \
{ \
{ \
- { Intrinsics::Bswap, false } \
+ INTRIN(Bswap, SideEffects_F, ReturnsTwice_F) \
, { Overload, Overload }, 2 \
} \
, "bswap." NameSuffix \
@@ -108,7 +118,7 @@
#define CtlzInit(Overload, NameSuffix) \
{ \
{ \
- { Intrinsics::Ctlz, false } \
+ INTRIN(Ctlz, SideEffects_F, ReturnsTwice_F) \
, { Overload, Overload, IceType_i1 }, 3 \
} \
, "ctlz." NameSuffix \
@@ -120,7 +130,7 @@
#define CtpopInit(Overload, NameSuffix) \
{ \
{ \
- { Intrinsics::Ctpop, false } \
+ INTRIN(Ctpop, SideEffects_F, ReturnsTwice_F) \
, { Overload, Overload }, 2 \
} \
, "ctpop." NameSuffix \
@@ -132,7 +142,7 @@
#define CttzInit(Overload, NameSuffix) \
{ \
{ \
- { Intrinsics::Cttz, false } \
+ INTRIN(Cttz, SideEffects_F, ReturnsTwice_F) \
, { Overload, Overload, IceType_i1 }, 3 \
} \
, "cttz." NameSuffix \
@@ -140,45 +150,57 @@
CttzInit(IceType_i32, "i32"),
CttzInit(IceType_i64, "i64"),
#undef CttzInit
- { { { Intrinsics::Longjmp, true },
+
+ { { INTRIN(Longjmp, SideEffects_T, ReturnsTwice_F),
{ IceType_void, IceType_i32, IceType_i32 }, 3 },
"nacl.longjmp" },
- { { { Intrinsics::Memcpy, true }, { IceType_void, IceType_i32, IceType_i32,
- IceType_i32, IceType_i32, IceType_i1 },
+ { { INTRIN(Memcpy, SideEffects_T, ReturnsTwice_F),
+ { IceType_void, IceType_i32, IceType_i32, IceType_i32, IceType_i32,
+ IceType_i1},
6 },
"memcpy.p0i8.p0i8.i32" },
- { { { Intrinsics::Memmove, true },
- { IceType_void, IceType_i32, IceType_i32,
- IceType_i32, IceType_i32, IceType_i1 },
+ { { INTRIN(Memmove, SideEffects_T, ReturnsTwice_F),
+ { IceType_void, IceType_i32, IceType_i32, IceType_i32, IceType_i32,
+ IceType_i1 },
6 },
"memmove.p0i8.p0i8.i32" },
- { { { Intrinsics::Memset, true }, { IceType_void, IceType_i32, IceType_i8,
- IceType_i32, IceType_i32, IceType_i1 },
+ { { INTRIN(Memset, SideEffects_T, ReturnsTwice_F),
+ { IceType_void, IceType_i32, IceType_i8, IceType_i32, IceType_i32,
+ IceType_i1 },
6 },
"memset.p0i8.i32" },
- { { { Intrinsics::NaClReadTP, false }, { IceType_i32 }, 1 },
+ { { INTRIN(NaClReadTP, SideEffects_F, ReturnsTwice_F),
+ { IceType_i32 }, 1 },
"nacl.read.tp" },
- { { { Intrinsics::Setjmp, true }, { IceType_i32, IceType_i32 }, 2 },
+ { { INTRIN(Setjmp, SideEffects_T, ReturnsTwice_T),
+ { IceType_i32, IceType_i32 }, 2 },
"nacl.setjmp" },
#define SqrtInit(Overload, NameSuffix) \
{ \
{ \
- { Intrinsics::Sqrt, false } \
- , { Overload, Overload }, 2 \
- } \
- , "sqrt." NameSuffix \
+ INTRIN(Sqrt, SideEffects_F, ReturnsTwice_F), \
+ { Overload, Overload }, 2 }, \
+ "sqrt." NameSuffix \
}
SqrtInit(IceType_f32, "f32"),
SqrtInit(IceType_f64, "f64"),
#undef SqrtInit
- { { { Intrinsics::Stacksave, true }, { IceType_i32 }, 1 }, "stacksave" },
- { { { Intrinsics::Stackrestore, true }, { IceType_void, IceType_i32 }, 2 },
+
+ { { INTRIN(Stacksave, SideEffects_T, ReturnsTwice_F),
+ { IceType_i32 }, 1 },
+ "stacksave" },
+ { { INTRIN(Stackrestore, SideEffects_T, ReturnsTwice_F),
+ { IceType_void, IceType_i32 }, 2 },
"stackrestore" },
- { { { Intrinsics::Trap, true }, { IceType_void }, 1 }, "trap" }
- };
+ { { INTRIN(Trap, SideEffects_T, ReturnsTwice_F),
+ { IceType_void }, 1 },
+ "trap" }
+};
const size_t IceIntrinsicsTableSize = llvm::array_lengthof(IceIntrinsicsTable);
+#undef INTRIN
+
} // end of anonymous namespace
Intrinsics::Intrinsics() {
diff --git a/src/IceIntrinsics.h b/src/IceIntrinsics.h
index b9c793a..d984b4c 100644
--- a/src/IceIntrinsics.h
+++ b/src/IceIntrinsics.h
@@ -88,15 +88,24 @@
static bool VerifyMemoryOrder(uint64_t Order);
+ enum SideEffects {
+ SideEffects_F=0,
+ SideEffects_T=1
+ };
+
+ enum ReturnsTwice {
+ ReturnsTwice_F=0,
+ ReturnsTwice_T=1
+ };
+
// Basic attributes related to each intrinsic, that are relevant to
- // code generation. We will want to have more attributes (e.g., Setjmp
- // returns twice and which affects stack coloring) once the lowering
- // cares about such attributes. Perhaps the attributes representation
- // can be shared with general function calls, though most functions
- // will be opaque.
+ // code generation. Perhaps the attributes representation can be shared
+ // with general function calls, but PNaCl currently strips all
+ // attributes from functions.
struct IntrinsicInfo {
- IntrinsicID ID : 31;
- bool HasSideEffects : 1;
+ enum IntrinsicID ID : 30;
+ enum SideEffects HasSideEffects : 1;
+ enum ReturnsTwice ReturnsTwice : 1;
};
// The complete set of information about an intrinsic.
diff --git a/src/IceTargetLowering.cpp b/src/IceTargetLowering.cpp
index 71788df..4ab6c63 100644
--- a/src/IceTargetLowering.cpp
+++ b/src/IceTargetLowering.cpp
@@ -171,9 +171,13 @@
case Inst::InsertElement:
lowerInsertElement(llvm::dyn_cast<InstInsertElement>(Inst));
break;
- case Inst::IntrinsicCall:
- lowerIntrinsicCall(llvm::dyn_cast<InstIntrinsicCall>(Inst));
+ case Inst::IntrinsicCall: {
+ InstIntrinsicCall *Call = llvm::dyn_cast<InstIntrinsicCall>(Inst);
+ if (Call->getIntrinsicInfo().ReturnsTwice)
+ setCallsReturnsTwice(true);
+ lowerIntrinsicCall(Call);
break;
+ }
case Inst::Load:
lowerLoad(llvm::dyn_cast<InstLoad>(Inst));
break;
diff --git a/src/IceTargetLowering.h b/src/IceTargetLowering.h
index a3410eb..b15c4f1 100644
--- a/src/IceTargetLowering.h
+++ b/src/IceTargetLowering.h
@@ -141,6 +141,12 @@
virtual llvm::ArrayRef<uint8_t> getNonExecBundlePadding() const = 0;
bool hasComputedFrame() const { return HasComputedFrame; }
bool shouldDoNopInsertion() const;
+ // Returns true if this function calls a function that has the
+ // "returns twice" attribute.
+ bool callsReturnsTwice() const { return CallsReturnsTwice; }
+ void setCallsReturnsTwice(bool RetTwice) {
+ CallsReturnsTwice = RetTwice;
+ }
int32_t getStackAdjustment() const { return StackAdjustment; }
void updateStackAdjustment(int32_t Offset) { StackAdjustment += Offset; }
void resetStackAdjustment() { StackAdjustment = 0; }
@@ -176,7 +182,7 @@
protected:
TargetLowering(Cfg *Func)
: Func(Func), Ctx(Func->getContext()), HasComputedFrame(false),
- StackAdjustment(0) {}
+ CallsReturnsTwice(false), StackAdjustment(0) {}
virtual void lowerAlloca(const InstAlloca *Inst) = 0;
virtual void lowerArithmetic(const InstArithmetic *Inst) = 0;
virtual void lowerAssign(const InstAssign *Inst) = 0;
@@ -210,6 +216,7 @@
Cfg *Func;
GlobalContext *Ctx;
bool HasComputedFrame;
+ bool CallsReturnsTwice;
// StackAdjustment keeps track of the current stack offset from its
// natural location, as arguments are pushed for a function call.
int32_t StackAdjustment;
diff --git a/src/IceTargetLoweringX8632.cpp b/src/IceTargetLoweringX8632.cpp
index c3a6e4d..abc6b41 100644
--- a/src/IceTargetLoweringX8632.cpp
+++ b/src/IceTargetLoweringX8632.cpp
@@ -647,9 +647,13 @@
// frames. If SimpleCoalescing is true, then each "global" variable
// without a register gets its own slot, but "local" variable slots
// are reused across basic blocks. E.g., if A and B are local to
- // block 1 and C is local to block 2, then C may share a slot with A
- // or B.
- const bool SimpleCoalescing = true;
+ // block 1 and C is local to block 2, then C may share a slot with A or B.
+ //
+ // We cannot coalesce stack slots if this function calls a "returns twice"
+ // function. In that case, basic blocks may be revisited, and variables
+ // local to those basic blocks are actually live until after the
+ // called function returns a second time.
+ const bool SimpleCoalescing = !callsReturnsTwice();
size_t InArgsSizeBytes = 0;
size_t PreservedRegsSizeBytes = 0;
SpillAreaSizeBytes = 0;
diff --git a/tests_lit/llvm2ice_tests/returns_twice_no_coalesce.ll b/tests_lit/llvm2ice_tests/returns_twice_no_coalesce.ll
new file mode 100644
index 0000000..754a105
--- /dev/null
+++ b/tests_lit/llvm2ice_tests/returns_twice_no_coalesce.ll
@@ -0,0 +1,60 @@
+; This file checks that SimpleCoalescing of local stack slots is not done
+; when calling a function with the "returns twice" attribute.
+
+; RUN: %llvm2ice -Om1 --verbose none %s \
+; RUN: | llvm-mc -triple=i686-none-nacl -x86-asm-syntax=intel -filetype=obj \
+; RUN: | llvm-objdump -d --symbolize -x86-asm-syntax=intel - | FileCheck %s
+; RUN: %llvm2ice --verbose none %s | FileCheck --check-prefix=ERRORS %s
+
+; Setjmp is a function with the "returns twice" attribute.
+declare i32 @llvm.nacl.setjmp(i8*)
+
+declare i32 @other(i32)
+declare void @user(i32)
+
+define i32 @call_returns_twice(i32 %iptr_jmpbuf, i32 %x) {
+entry:
+ %local = add i32 %x, 12345
+ %jmpbuf = inttoptr i32 %iptr_jmpbuf to i8*
+ %y = call i32 @llvm.nacl.setjmp(i8* %jmpbuf)
+ call void @user(i32 %local)
+ %cmp = icmp eq i32 %y, 0
+ br i1 %cmp, label %Zero, label %NonZero
+Zero:
+ %other_local = add i32 %x, 54321
+ call void @user(i32 %other_local)
+ ret i32 %other_local
+NonZero:
+ ret i32 1
+}
+
+; CHECK-LABEL: call_returns_twice
+; CHECK: add [[REG1:.*]], 12345
+; CHECK: mov dword ptr [esp + [[OFF:.*]]], [[REG1]]
+; CHECK: add [[REG2:.*]], 54321
+; There should not be sharing of the stack slot.
+; CHECK-NOT: mov dword ptr [esp + [[OFF]]], [[REG2]]
+
+define i32 @no_call_returns_twice(i32 %iptr_jmpbuf, i32 %x) {
+entry:
+ %local = add i32 %x, 12345
+ %y = call i32 @other(i32 %x)
+ call void @user(i32 %local)
+ %cmp = icmp eq i32 %y, 0
+ br i1 %cmp, label %Zero, label %NonZero
+Zero:
+ %other_local = add i32 %x, 54321
+ call void @user(i32 %other_local)
+ ret i32 %other_local
+NonZero:
+ ret i32 1
+}
+
+; CHECK-LABEL: no_call_returns_twice
+; CHECK: add [[REG1:.*]], 12345
+; CHECK: mov dword ptr [esp + [[OFF:.*]]], [[REG1]]
+; CHECK: add [[REG2:.*]], 54321
+; Now there should be sharing of the stack slot (OFF is the same).
+; CHECK: mov dword ptr [esp + [[OFF]]], [[REG2]]
+
+; ERRORS-NOT: ICE translation error