[PATCH] D144472: [InstCombine][debuginfo] Update valueCoversEntireFragment for fixed size fragment and scalable value.

[Yeting Kuo via Phabricator via llvm-commits]

fakepaper56 added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/Local.cpp:1498-1499
+  if (std::optional<uint64_t> FragmentSize = DII->getFragmentSizeInBits())
+    return !ValueSize.isScalable() &&
+           ValueSize.getFixedValue() >= *FragmentSize;
+
----------------
sdesmalen wrote:
> I'm not sure how `valueCoversEntireFragment` is used, but would it make more sense to write: 
> 
>   return TypeSize::isKnownGE(ValueSize, TypeSize::getFixed(*FragmentSize)
> 
> ?
Good point. Thank your reminder. I also add tests for converting `gdb.declare` to `gdb.value`.


================
Comment at: llvm/test/Transforms/InstCombine/dbg-scalable-store-fixed-frag.ll:1
+; RUN: opt < %s -passes='instcombine' -S
+
----------------
sdesmalen wrote:
> fakepaper56 wrote:
> > sdesmalen wrote:
> > > There are no CHECK-lines and the running opt on this file doesn't cause an assertion failure for me. What is this test supposed to verify?
> > Sorry that I update the test. It should fail by assertion. 
> Thanks, I can confirm the test now fails for me.
Thank you for your check.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144472