Update valueCoversEntireFragment to handle new case and add regression test.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
| llvm/test/Transforms/InstCombine/dbg-scalable-store-fixed-frag.ll | ||
|---|---|---|
| 2 | 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? | |
| llvm/test/Transforms/InstCombine/dbg-scalable-store-fixed-frag.ll | ||
|---|---|---|
| 2 | Sorry that I update the test. It should fail by assertion. | |
| llvm/lib/Transforms/Utils/Local.cpp | ||
|---|---|---|
| 1497–1498 | I'm not sure how valueCoversEntireFragment is used, but would it make more sense to write: return TypeSize::isKnownGE(ValueSize, TypeSize::getFixed(*FragmentSize) ? | |
| llvm/test/Transforms/InstCombine/dbg-scalable-store-fixed-frag.ll | ||
| 2 | Thanks, I can confirm the test now fails for me. | |
| llvm/test/Transforms/InstCombine/dbg-scalable-store-fixed-frag.ll | ||
|---|---|---|
| 26 | This IR isn't well-formed and a future verifier might reject it. Can you created two distinct DISubprograms and local variables for each of the functions? | |
| llvm/test/Transforms/InstCombine/dbg-scalable-store-fixed-frag.ll | ||
|---|---|---|
| 26 | Thank your good point. I have fixed it. | |
Seems fine to me, but please give @aprantl a chance to have another look at it as well before you land it.
I'm not sure how valueCoversEntireFragment is used, but would it make more sense to write:
?