[PATCH] D57033: [LCSSA] Handle case with single new PHI faster.

Florian Hahn via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jan 24 15:03:50 PST 2019


fhahn marked an inline comment as done.
fhahn added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/LCSSA.cpp:226
+      Value *V = AddedPHIs.size() == 1 ? AddedPHIs[0]
+                                       : SSAUpdate.FindValueForBlock(UserBB);
+      DVI->setOperand(0, MetadataAsValue::get(Ctx, ValueAsMetadata::get(V)));
----------------
fhahn wrote:
> efriedma wrote:
> > fhahn wrote:
> > > efriedma wrote:
> > > > I'm not sure I understand why you're changing the logic here for the `AddedPHIs.size() != 1` case.
> > > In the AddedPHIs.size() == 1 case, SSAUpdate won't be used to rewrite the uses, so we can just use the only inserted PHI.
> > Let me rephrase: the setOperand() call used to be guarded by an if statement. Why are you making the call unconditional?
> Ah sorry, I missed your point!
> 
> I was just going by the comment, which I think is not completely true. We currently handle debug values residing in blocks that are part of the subset traversed from renamed uses to defs (those traversed by SSAUpdater while rewriting). But debug values could also be used outside this subset, so we need the conditional. I've updated the comment and added it back.
Oh and I'll add a test case illustrating that (seems to be missing ATM tomorrow ); it's too late on my side of the planet :)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57033/new/

https://reviews.llvm.org/D57033





More information about the llvm-commits mailing list