Skip to content
This repository has been archived by the owner on Nov 1, 2021. It is now read-only.

Commit

Permalink
LLVM's stripPointerCasts can now look through a call/invoke (possibly…
Browse files Browse the repository at this point in the history 
… with side effects) if it has a param with the 'returned' attribute
  • Loading branch information
@kripken
kripken committed Jan 24, 2017
1 parent 39715d8 commit 63e73b3
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 7 deletions.
2 changes: 1 addition & 1 deletion lib/Target/JSBackend/CallHandlers.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ const Value *getActuallyCalledValue(const Instruction *I) {
// for example, extern void x() in C will turn into void x(...) in LLVM IR, then the IR bitcasts
// it to the proper form right before the call. this both causes an unnecessary indirect
// call, and it is done with the wrong type. TODO: don't even put it into the function table
if (const Function *F = dyn_cast<const Function>(CV->stripPointerCasts())) {
if (const Function *F = dyn_cast<const Function>(stripPointerCastsWithoutSideEffects(CV))) {
CV = F;
}
return CV;
Expand Down
25 changes: 19 additions & 6 deletions lib/Target/JSBackend/JSBackend.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,19 @@ namespace {
raw_pwrite_stream& nl(raw_pwrite_stream &Out, int delta = 0);

private:

This comment has been minimized.

Sign in to view

Copy link
@kripken

kripken Jan 24, 2017

Author Member

@sunfishcode - hey, this LLVM change surprised me, see comment right below. Did LLVM really change stripPointerCast semantics that much, that it can now look through a call/invoke? Seems surprising to me, as the LLVM include header still says this:

  /// \brief Strip off pointer casts, all-zero GEPs, and aliases.
  ///
  /// Returns the original uncasted value.  If this is called on a non-pointer
  /// value, it returns 'this'.

Is a call/invoke instruction that has a returned argument really a "pointer cast" in LLVM terms? Also surprising to me is that this changed in LLVM in the Value.cpp part of this commit:, which seems kind of offhand, 4cb3366de0#diff-fd8ef775c4b6fe99f60d2a20f95f6b80R467

This comment has been minimized.

Sign in to view

Copy link
@sunfishcode

sunfishcode Jan 24, 2017

Collaborator

Looks like the answer is yes, LLVM really did do that. That's a little surprising. I guess there are two kinds of users of stripPointerCast, those that just want to ignore "no-op" casts, and those that want to get as close as they can to the real definition of a value, and it looks like the latter users motivated this change.

This comment has been minimized.

Sign in to view

Copy link
@sunfishcode

sunfishcode Jan 24, 2017

Collaborator

Ah, and along with this change is a new function, stripPointerCastsWithoutSideEffects, which seems like it might be useful for you.

This comment has been minimized.

Sign in to view

Copy link
@kripken

kripken Jan 24, 2017

Author Member

Cool, thanks for confirming it's not just weird to me.

Sadly that new method isn't in 4.0 (or was added very recently to the branch). I rolled my own.
// LLVM changed stripPointerCasts to use the "returned" attribute on
// calls and invokes, i.e., stripping pointer casts of a call to
// define internal i8* @strupr(i8* returned %str) #2 {
// will return the pointer, and ignore the call which has side
// effects. We sometimes do care about the side effects.
const Value* stripPointerCastsWithoutSideEffects(const Value* V) {
if (isa<CallInst>(V) || isa<InvokeInst>(V)) {
return V; // in theory we could check if there actually are side effects
}
return V->stripPointerCasts();
}

void printCommaSeparated(const HeapData v);

// parsing of constants has two phases: calculate, and then emit
Expand Down Expand Up @@ -1679,7 +1692,7 @@ std::string JSWriter::getConstantVector(const ConstantVectorType *C) {

std::string JSWriter::getValueAsStr(const Value* V, AsmCast sign) {
// Skip past no-op bitcasts and zero-index geps.
V = V->stripPointerCasts();
V = stripPointerCastsWithoutSideEffects(V);

if (const Constant *CV = dyn_cast<Constant>(V)) {
return getConstant(CV, sign);
Expand All @@ -1690,7 +1703,7 @@ std::string JSWriter::getValueAsStr(const Value* V, AsmCast sign) {

std::string JSWriter::getValueAsCastStr(const Value* V, AsmCast sign) {
// Skip past no-op bitcasts and zero-index geps.
V = V->stripPointerCasts();
V = stripPointerCastsWithoutSideEffects(V);

if (isa<ConstantInt>(V) || isa<ConstantFP>(V)) {
return getConstant(cast<Constant>(V), sign);
Expand All @@ -1701,7 +1714,7 @@ std::string JSWriter::getValueAsCastStr(const Value* V, AsmCast sign) {

std::string JSWriter::getValueAsParenStr(const Value* V) {
// Skip past no-op bitcasts and zero-index geps.
V = V->stripPointerCasts();
V = stripPointerCastsWithoutSideEffects(V);

if (const Constant *CV = dyn_cast<Constant>(V)) {
return getConstant(CV);
Expand All @@ -1712,7 +1725,7 @@ std::string JSWriter::getValueAsParenStr(const Value* V) {

std::string JSWriter::getValueAsCastParenStr(const Value* V, AsmCast sign) {
// Skip past no-op bitcasts and zero-index geps.
V = V->stripPointerCasts();
V = stripPointerCastsWithoutSideEffects(V);

if (isa<ConstantInt>(V) || isa<ConstantFP>(V) || isa<UndefValue>(V)) {
return getConstant(cast<Constant>(V), sign);
Expand Down Expand Up @@ -2362,7 +2375,7 @@ void JSWriter::generateExpression(const User *I, raw_string_ostream& Code) {
// and all-zero-index geps that LLVM needs to satisfy its type system, we
// call stripPointerCasts() on all values before translating them. This
// includes bitcasts whose only use is lifetime marker intrinsics.
assert(I == I->stripPointerCasts());
assert(I == stripPointerCastsWithoutSideEffects(I));

Type *T = I->getType();
if (T->isIntegerTy() && ((!OnlyWebAssembly && T->getIntegerBitWidth() > 32) ||
Expand Down Expand Up @@ -2961,7 +2974,7 @@ void JSWriter::addBlock(const BasicBlock *BB, Relooper& R, LLVMToRelooperMap& LL
for (BasicBlock::const_iterator II = BB->begin(), E = BB->end();
II != E; ++II) {
auto I = &*II;
if (I->stripPointerCasts() == I) {
if (stripPointerCastsWithoutSideEffects(I) == I) {
CurrInstruction = I;
generateExpression(I, CodeStream);
}
Expand Down

0 comments on commit 63e73b3

Please sign in to comment.