Skip to content

[IRGen] Avoid generating mergeable traps #14729

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 20, 2018
Merged

[IRGen] Avoid generating mergeable traps #14729

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions lib/IRGen/GenCast.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -422,10 +422,7 @@ emitExistentialScalarCastFn(IRGenModule &IGM,
}

case CheckedCastMode::Unconditional: {
llvm::Function *trapIntrinsic = llvm::Intrinsic::getDeclaration(&IGM.Module,
llvm::Intrinsic::ID::trap);
IGF.Builder.CreateCall(trapIntrinsic, {});
IGF.Builder.CreateUnreachable();
IGF.emitTrap(/*EmitUnreachable=*/true);
break;
}
}
Expand Down
5 changes: 1 addition & 4 deletions lib/IRGen/GenProto.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1968,10 +1968,7 @@ llvm::Constant *WitnessTableBuilder::buildInstantiationFunction() {

// The counts didn't match; abort.
IGF.Builder.emitBlock(failBB);
llvm::Function *trapIntrinsic =
llvm::Intrinsic::getDeclaration(&IGM.Module, llvm::Intrinsic::ID::trap);
IGF.Builder.CreateCall(trapIntrinsic, {});
IGF.Builder.CreateUnreachable();
IGF.emitTrap(/*EmitUnreachable=*/true);

return fn;
}
Expand Down
25 changes: 25 additions & 0 deletions lib/IRGen/IRGenFunction.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -431,3 +431,28 @@ Address IRGenFunction::emitAddressAtOffset(llvm::Value *base, Offset offset,
auto slotPtr = emitByteOffsetGEP(base, offsetValue, objectTy);
return Address(slotPtr, objectAlignment);
}

void IRGenFunction::emitTrap(bool EmitUnreachable) {
if (IGM.IRGen.Opts.shouldOptimize()) {
// Emit unique side-effecting inline asm calls in order to eliminate
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be the Doxygen comment for the function?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO this describes the implementation details at too low a level. The doxygen comment I added in the header file describes the high-level effect more succinctly.

// the possibility that an LLVM optimization or code generation pass
// will merge these blocks back together again. We emit an empty asm
// string with the side-effect flag set, and with a unique integer
// argument for each cond_fail we see in the function.
llvm::IntegerType *asmArgTy = IGM.Int32Ty;
llvm::Type *argTys = {asmArgTy};
llvm::FunctionType *asmFnTy =
llvm::FunctionType::get(IGM.VoidTy, argTys, false /* = isVarArg */);
llvm::InlineAsm *inlineAsm =
llvm::InlineAsm::get(asmFnTy, "", "n", true /* = SideEffects */);
Builder.CreateAsmCall(inlineAsm,
llvm::ConstantInt::get(asmArgTy, NumTrapBarriers++));
}

// Emit the trap instruction.
llvm::Function *trapIntrinsic =
llvm::Intrinsic::getDeclaration(&IGM.Module, llvm::Intrinsic::ID::trap);
Builder.CreateCall(trapIntrinsic, {});
if (EmitUnreachable)
Builder.CreateUnreachable();
}
4 changes: 4 additions & 0 deletions lib/IRGen/IRGenFunction.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,11 @@ class IRGenFunction {
/// Mark a load as dereferenceable to `size` bytes.
void setDereferenceableLoad(llvm::LoadInst *load, unsigned size);

/// Emit a non-mergeable trap call, optionally followed by a terminator.
void emitTrap(bool EmitUnreachable);

private:
unsigned NumTrapBarriers = 0;
llvm::Instruction *AllocaIP;
const SILDebugScope *DbgScope;

Expand Down
33 changes: 3 additions & 30 deletions lib/IRGen/IRGenSIL.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -388,7 +388,6 @@ class IRGenSILFunction :
/// Holds the DominancePoint of values that are storage for a source variable.
SmallVector<std::pair<llvm::Instruction *, DominancePoint>, 8> ValueDomPoints;
unsigned NumAnonVars = 0;
unsigned NumCondFails = 0;

/// Accumulative amount of allocated bytes on the stack. Used to limit the
/// size for stack promoted objects.
Expand Down Expand Up @@ -3851,9 +3850,7 @@ static bool hasReferenceSemantics(IRGenSILFunction &IGF,
static llvm::Value *emitIsUnique(IRGenSILFunction &IGF, SILValue operand,
SourceLoc loc, bool checkPinned) {
if (!hasReferenceSemantics(IGF, operand->getType())) {
llvm::Function *trapIntrinsic = llvm::Intrinsic::getDeclaration(
&IGF.IGM.Module, llvm::Intrinsic::ID::trap);
IGF.Builder.CreateCall(trapIntrinsic, {});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a way we could prevent this from being called by accident in new code, maybe by hiding access to that particular intrinsic behind a private method? It may not be feasible because it is just modeled as a call...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's possible to interpose a custom builder over llvm::IRBuilder, e.g clang does it. And if we can do that, we can prevent calls to ID::trap from being created. I'll look into it.

IGF.emitTrap(/*EmitUnreachable=*/false);
return llvm::UndefValue::get(IGF.IGM.Int1Ty);
}

Expand Down Expand Up @@ -4480,10 +4477,7 @@ static void emitTrapAndUndefValue(IRGenSILFunction &IGF,
IGF.FailBBs.push_back(failBB);

IGF.Builder.emitBlock(failBB);
llvm::Function *trapIntrinsic = llvm::Intrinsic::getDeclaration(
&IGF.IGM.Module, llvm::Intrinsic::ID::trap);
IGF.Builder.CreateCall(trapIntrinsic, {});
IGF.Builder.CreateUnreachable();
IGF.emitTrap(/*EmitUnreachable=*/true);

llvm::BasicBlock *contBB = llvm::BasicBlock::Create(IGF.IGM.getLLVMContext());
IGF.Builder.emitBlock(contBB);
Expand Down Expand Up @@ -5377,28 +5371,7 @@ void IRGenSILFunction::visitCondFailInst(swift::CondFailInst *i) {
llvm::BasicBlock *contBB = llvm::BasicBlock::Create(IGM.getLLVMContext());
Builder.CreateCondBr(cond, failBB, contBB);
Builder.emitBlock(failBB);

if (IGM.IRGen.Opts.shouldOptimize()) {
// Emit unique side-effecting inline asm calls in order to eliminate
// the possibility that an LLVM optimization or code generation pass
// will merge these blocks back together again. We emit an empty asm
// string with the side-effect flag set, and with a unique integer
// argument for each cond_fail we see in the function.
llvm::IntegerType *asmArgTy = IGM.Int32Ty;
llvm::Type *argTys = { asmArgTy };
llvm::FunctionType *asmFnTy =
llvm::FunctionType::get(IGM.VoidTy, argTys, false /* = isVarArg */);
llvm::InlineAsm *inlineAsm =
llvm::InlineAsm::get(asmFnTy, "", "n", true /* = SideEffects */);
Builder.CreateAsmCall(inlineAsm,
llvm::ConstantInt::get(asmArgTy, NumCondFails++));
}

// Emit the trap instruction.
llvm::Function *trapIntrinsic =
llvm::Intrinsic::getDeclaration(&IGM.Module, llvm::Intrinsic::ID::trap);
Builder.CreateCall(trapIntrinsic, {});
Builder.CreateUnreachable();
emitTrap(/*EmitUnreachable=*/true);
Builder.emitBlock(contBB);
FailBBs.push_back(failBB);
}
Expand Down
6 changes: 6 additions & 0 deletions test/IRGen/bitcast_different_size.sil
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
// RUN: %target-swift-frontend -assume-parsing-unqualified-ownership-sil -emit-ir %s -verify | %FileCheck %s
// RUN: %target-swift-frontend -assume-parsing-unqualified-ownership-sil -emit-ir -O %s -verify | %FileCheck %s --check-prefix=OPT

// REQUIRES: CPU=i386 || CPU=x86_64

Expand All @@ -8,6 +9,11 @@ import Swift

// CHECK-LABEL: define{{( protected)?}} swiftcc i64 @bitcast_different_size1

// OPT-LABEL: define{{.*}}@bitcast_different_size1(i32)
// OPT: tail call void asm sideeffect "", "n"(i32 0) #2
// OPT-NEXT: tail call void @llvm.trap()
// OPT-NEXT: unreachable

sil @bitcast_different_size1 : $@convention(thin) (Int32) -> Int64 {
entry(%i : $Int32):
// CHECK: ret {{.*}}undef
Expand Down