[PATCH] D90046: [LiveDebugValues] Handle spill locations with a fixed and scalable component.

Mon Jan 11 13:41:27 PST 2021

jmorse added inline comments.

================
Comment at: llvm/lib/CodeGen/LiveDebugValues/VarLocBasedImpl.cpp:574-576
+                   Other.Var, Other.Kind, Other.Loc.SpillLocation.SpillBase,
+                   Loc.SpillLocation.SpillOffset.getFixed(),
+                   Loc.SpillLocation.SpillOffset.getScalable(), Other.Expr);
----------------
sdesmalen wrote:
> StephenTozer wrote:
> > The SpillOffset fixed and scalable arguments here are missing `Other.`, so we're comparing the current `VarLoc` to itself - can you fix this in a followup patch?
> Good spot, thanks for pointing out! Do you have any objection for me to just push the fix directly? (I wouldn't really know how to create a test for this, unless you have any suggestions?)
IMO just pushing up the fix is fine -- the developer policy permits it if it's "obvious" that the patch is correct.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90046