Please use GitHub pull requests for new patches. Avoid migrating existing patches. Phabricator shutdown timeline
Changeset View
Changeset View
Standalone View
Standalone View
llvm/lib/Analysis/PHITransAddr.cpp
| Show First 20 Lines • Show All 171 Lines • ▼ Show 20 Lines | Value *PHITransAddr::translateSubExpr(Value *V, BasicBlock *CurBB, | ||||
| if (CastInst *Cast = dyn_cast<CastInst>(Inst)) { | if (CastInst *Cast = dyn_cast<CastInst>(Inst)) { | ||||
| Value *PHIIn = translateSubExpr(Cast->getOperand(0), CurBB, PredBB, DT); | Value *PHIIn = translateSubExpr(Cast->getOperand(0), CurBB, PredBB, DT); | ||||
| if (!PHIIn) return nullptr; | if (!PHIIn) return nullptr; | ||||
| if (PHIIn == Cast->getOperand(0)) | if (PHIIn == Cast->getOperand(0)) | ||||
| return Cast; | return Cast; | ||||
| // Find an available version of this cast. | // Find an available version of this cast. | ||||
| // Constants are trivial to find. | // Try to simplify cast first. | ||||
| if (Constant *C = dyn_cast<Constant>(PHIIn)) | if (Value *V = simplifyCastInst(Cast->getOpcode(), PHIIn, Cast->getType(), | ||||
| return addAsInput( | {DL, TLI, DT, AC})) { | ||||
| ConstantExpr::getCast(Cast->getOpcode(), C, Cast->getType())); | RemoveInstInputs(PHIIn, InstInputs); | ||||
| return addAsInput(V); | |||||
| } | |||||
mkazantsev: I'm fine with `isSafeToSpeculativelyExecute` part, it seems reasonable. But this doesn't seem… | |||||
Not Done ReplyInline ActionsI agree, after this change it can fold zext/sext + trunc for example (didn't see any changes on test-suite though). kachkov98: I agree, after this change it can fold zext/sext + trunc for example (didn't see any changes on… | |||||
Not Done ReplyInline ActionsCan you write a test yourself? mkazantsev: Can you write a test yourself? | |||||
| // Otherwise we have to see if a casted version of the incoming pointer | // Otherwise we have to see if a casted version of the incoming pointer | ||||
| // is available. If so, we can use it, otherwise we have to fail. | // is available. If so, we can use it, otherwise we have to fail. | ||||
| for (User *U : PHIIn->users()) { | for (User *U : PHIIn->users()) { | ||||
| if (CastInst *CastI = dyn_cast<CastInst>(U)) | if (CastInst *CastI = dyn_cast<CastInst>(U)) | ||||
| if (CastI->getOpcode() == Cast->getOpcode() && | if (CastI->getOpcode() == Cast->getOpcode() && | ||||
| CastI->getType() == Cast->getType() && | CastI->getType() == Cast->getType() && | ||||
| (!DT || DT->dominates(CastI->getParent(), PredBB))) | (!DT || DT->dominates(CastI->getParent(), PredBB))) | ||||
| ▲ Show 20 Lines • Show All 228 Lines • Show Last 20 Lines | |||||
I'm fine with isSafeToSpeculativelyExecute part, it seems reasonable. But this doesn't seem to be NFC while whole patch claims it's an NFC. Should it be 2 different patches?