[PATCH] D107233: [SimplifyCFG] Enable switch to lookup table for more types.

Roman Lebedev via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Aug 3 07:09:42 PDT 2021


lebedev.ri added a comment.

In D107233#2922229 <https://reviews.llvm.org/D107233#2922229>, @fhahn wrote:

> In D107233#2922212 <https://reviews.llvm.org/D107233#2922212>, @fhahn wrote:
>
>> In D107233#2922195 <https://reviews.llvm.org/D107233#2922195>, @lebedev.ri wrote:
>>
>>> In D107233#2922165 <https://reviews.llvm.org/D107233#2922165>, @fhahn wrote:
>>>
>>>> In D107233#2918643 <https://reviews.llvm.org/D107233#2918643>, @lebedev.ri wrote:
>>>>
>>>>> Can we just revert rL168970 <https://reviews.llvm.org/rL168970>?
>>>>> What exactly was the `rdar://12779436` about, can anyone look it up?
>>>>
>>>> It's an Apple-internal bug report of a crash that was the reason for the revert. As long as the test that has been added in rL168970 <https://reviews.llvm.org/rL168970> passes that should be fine. I can also verify against the original C++ source.
>>>
>>> Note that most people don't have access to any such internal bugtrackers.
>>> What was the crast?
>>
>> I guess in this case, the reference is a historical artifact. I assume the test added to `llvm/test/Transforms/SimplifyCFG/X86/switch_to_lookup_table.ll` in the commit should be sufficient to guard against the reported issue.
>
> The crash was in the AArch64 backend due to the emitted table using un-emittable constants. I don't really have any further insight and cannot say if it is still an issue. I can check a patch against the original source to check if it crashes.

Aha. But that was not a reasonable fix. It would still crash just the same should anyone produce such an IR,
the fix should have been in the backend, and indeed, it no longer crashes, for a long while now: https://godbolt.org/z/34nf7q5xz
So back to my original question: can we just essentially revert that commit?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107233



More information about the llvm-commits mailing list