[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