[PATCH] D150943: [SimplifyCFG] add nsw on BuildLookuptable LinearMap calculation

Mikael Holmén via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Sep 8 03:52:22 PDT 2023


uabelho added a comment.

Hi @khei4 and @nikic

I think I've found a case where this patch causes a miscompile.
If we do

  opt bbi-86196.ll -o - -S -passes="simplifycfg<switch-to-lookup>"

on

  define i8 @foo(i3 %0) {                              ; Assume 1
  entry:
    switch i3 %0, label %cond.end [
      i3 -1, label %cond.false
      i3 -2, label %cond.false
      i3 1, label %cond.false                          ; -> %cond.false
      i3 0, label %cond.false
    ]
  
  cond.false:                                       ; preds = %entry, %entry, %entry, %entry
    %mul = shl nsw i3 %0, 1                            ; 1 << 1 = 2
    br label %cond.end
  
  cond.end:                                         ; preds = %entry, %cond.false
    %cond = phi i3 [ %mul, %cond.false ], [ 2, %entry ]; 2
    %conv = sext i3 %cond to i8
    ret i8 %conv
  }

we get

  define i16 @foo(i3 %0) {                            ; Assume 1
  entry:
    %switch.tableidx = sub i3 %0, -2                  ; 1 - -2 = 3
    %1 = icmp ult i3 %switch.tableidx, -4             ; 3 (011) u< -4 (100) = 1
    %switch.idx.mult = mul nsw i3 %switch.tableidx, 2 ; 3 * 2 = 6 ovf -2 poison?!
    %switch.offset = add nsw i3 %switch.idx.mult, -4  ; -2 + -4 = -6 ovf 2 poison?!
    %cond = select i1 %1, i3 %switch.offset, i3 2     ; %1 is 1 -> 2 poison?!
    %conv = sext i3 %cond to i16                      ; 2 poison
    ret i16 %conv                                     ; 2 poison
  }

which I think is wrong.
I've added some comments in the input and output showing what I think happens for the input value 1.
There are "nsw" on the mul and add instructions, but both caluculations overflow the i3 so I think we get poison there after the transformation?

F29141361: bbi-86196.ll <https://reviews.llvm.org/F29141361>


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150943



More information about the llvm-commits mailing list