[PATCH] D107017: [GlobalISel] Allow the ArtifactValueFinder to return the best available register on failure.

[Amara Emerson via Phabricator via llvm-commits]

aemerson added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/GlobalISel/LegalizationArtifactCombiner.h:756-759
+    /// Try to find a source of the value defined in the def \p DefReg, starting
+    /// at position \p StartBit with size \p Size.
+    /// \returns an empty Register if no value could be found, or \p DefReg if
+    /// if that was the best we could do.
----------------
foad wrote:
> aemerson wrote:
> > foad wrote:
> > > Surely DefReg is always a safe "best" return value? Why would you ever need to return Register() instead?
> > I suppose it's a matter of API design/opinion. If you get a valid Register back, then you know it's going to be different from the existing register so you can RAUW it. If it can return DefReg, then you need to check if it's different first, otherwise you'll trigger lots of unnecessary observer invalidations.
> > If you get a valid Register back, then you know it's going to be different from the existing register
> 
> I'm confused: the comment specifically says that you might get DefReg back, which is what you passed in.
I think that's an obsolete comment, and I actually need to change the users to reflect the semantics.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107017