[PATCH] D126525: [SSAUpdaterImpl] Do not generate phi node with all the same incoming values

[Sameer Sahasrabuddhe via Phabricator via llvm-commits]

sameerds added a subscriber: arsenm.
sameerds added a comment.

Requesting @arsenm to take a look at the AMDGPU test.



================
Comment at: llvm/include/llvm/Transforms/Utils/SSAUpdaterImpl.h:332
+    BBInfo *PredInfo = Info->Preds[0];
+    assert(PredInfo && "Pred Info?");
+    if (!PredInfo->AvailableVal)
----------------
Is this assert really needed? For someone unfamiliar with this code (like me), seeing this assert creates a doubt about the surrounding code ... as if there are complicated paths reaching here, where it might be possible that the BBInfo //may// be in an invalid state at this point.


================
Comment at: llvm/include/llvm/Transforms/Utils/SSAUpdaterImpl.h:338
+      BBInfo *PredInfo = Info->Preds[Idx];
+      assert(PredInfo && "Pred Info?");
+      if (!PredInfo->AvailableVal || Singular != PredInfo->AvailableVal)
----------------
Is this assert really needed?


================
Comment at: llvm/include/llvm/Transforms/Utils/SSAUpdaterImpl.h:347
+    BBMap[Info->BB]->AvailableVal = Singular;
+    assert(Info->AvailableVal && "Missed assignement?");
+    return true;
----------------
When is "Info->AvailableVal" assigned? If it's the same as the previous statement, then the implicit identity relationship between BBMap[Info->BB] and Info is quite confusing. Can this function be rewritten to consistently use just one of these two expressions?


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

https://reviews.llvm.org/D126525