leonardchan added a comment.

Almost there! Just a few more comments on my end.



================
Comment at: llvm/lib/Transforms/Utils/SimplifyCFG.cpp:5694
+    Constant *CaseRes = Values[I].second;
+    if (auto *GlobalVal = dyn_cast<GlobalValue>(CaseRes))
+      if (!GlobalVal->isDSOLocal())
----------------
I think we should also return false if it doesn't turn out to be a GlobalValue 
here.


================
Comment at: 
llvm/test/Transforms/SimplifyCFG/X86/switch_to_relative_lookup_table.ll:201-203
+; If there is a lookup table, where each element contains the same value,
+; a relative lookup should not be generated
+define void @single_value(i32 %cond)  {
----------------
It looks like this might not be testing what the comment says it should. It 
looks like in the phi statements below that each of the cases have different 
values. I think maybe you'll want something like:

```
  %str1.0 = phi i8* [ getelementptr inbounds ([5 x i8], [5 x i8]* @.str, i64 0, 
i64 0), %sw.default ], [ getelementptr inbounds ([5 x i8], [5 x i8]* @.str, i64 
0, i64 0), %sw.bb2 ], [ getelementptr inbounds ([5 x i8], [5 x i8]* @.str, i64 
0, i64 0), %sw.bb1 ], [ getelementptr inbounds ([5 x i8], [5 x i8]* @.str, i64 
0, i64 0), %entry ]
  ret i8* %str1.0  ; instead of `ret void` to ensure the `%str1.0` isn't 
optimized out
```

Then ensure this just lowers directly to a GEP for `@.str` and no lookup table 
is generated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94355

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to