jdoerfert added a comment.

In D106301#2888203 <https://reviews.llvm.org/D106301#2888203>, @JonChesterfield 
wrote:

> In D106301#2888170 <https://reviews.llvm.org/D106301#2888170>, @jdoerfert 
> wrote:
>
>> llvm.trap is preserved, thus branches to an llvm.trap are preserved.
>
> That's interesting. Consistent with IR in general,
>
>   template <bool Trap> int test(int x) {
>     if (x < 42) {
>       return x;
>     } else {
>       if (Trap)
>         __builtin_trap();
>       __builtin_unreachable();
>     }
>   }
>   
>   extern "C" {
>   int trap(int x) { return test<true>(x); }
>   int none(int x) { return test<false>(x); }
>   }
>
> `=>`
>
>   define i32 @trap(i32 returned %0) {
>     %2 = icmp slt i32 %0, 42
>     br i1 %2, label %4, label %3
>   
>   3:                                                ; preds = %1
>     tail call void @llvm.trap() #3
>     unreachable
>   
>   4:                                                ; preds = %1
>     ret i32 %0
>   }
>   
>   define i32 @none(i32 returned %0)  {
>     %2 = icmp slt i32 %0, 42
>     tail call void @llvm.assume(i1 %2) #3
>     ret i32 %0
>   }
>
> So yes, we'll get faster codegen if we are willing to throw away traps 
> followed by unreachable code.
>
> If that's a legitimate transform to do, it seems like something we should do 
> in instcombine, instead of a separate pass. I.e. fold `trap, unreachable` to 
> `unreachable`.
>
> Can we do that instead?

We could, but we should not. A trap inserted by assert or the user should stay 
(IMHO).
What we do here is to avoid new traps that were arbitrarily inserted with 
unreachables. There is no particular reason why some "reasons" for an 
unreachable insert a trap
and others do not, it's just "to help debugging".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106301

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

Reply via email to