[llvm] [SimplifyCFG] When only one case value is missing, replace default with that case (PR #76669)
Jeremy Morse via llvm-commits
llvm-commits at lists.llvm.org
Mon Jan 1 18:55:11 PST 2024
https://github.com/jmorse commented:
What's the purpose of adding the profile-data to the test case that's been updated, does it not work without that?
The original author apparently wished there to be a negative test case, if it's possible to keep a negative case and just add another test function for the new behaviour, IMHO that's preferred. (Perhaps that's not possible, but it would mean that the test coverage strictly improves).
The change here looks good -- however all the rationale for why this change is being made is in the rust bug report you link to. Could you please elaborate the comment on the added code ("When only one case value is missing...") to explain why this is a good thing to do. There's a risk that someone in the future tries to delete the code if it's not clear to them why it's beneficial.
To confirm my own understanding: adding an unreachable default is fine because we know the bits being switched upon are in a small finite set of cases; the new form has a better structure / is easier to codegen for? If so, sounds like a good change, LGTM.
https://github.com/llvm/llvm-project/pull/76669
More information about the llvm-commits
mailing list