[InstCombiner] WRONG code, InstCombiner suspected

[JonPsson1]

Bisect:
f044564 is the first bad commit

commit f044564 "[InstCombine] Make backedge check in op of phi transform more precise (#106075)"

clang -march=z15 -O1 wrong0.i
wrong0.tar.gz

I could not revert this commit cleanly on main, unfortunately - the diff below is between f044564^ <> f044564.

It looks to me that the inserted constant in the PHI should be 1 and not 0, or?:

cond.end:                                         ; preds = %cond.false, %      cond.end:                                         ; preds = %cond.false, %
  %cond = phi i64 [ %1, %cond.true ], [ %div, %cond.false ]                       %cond = phi i64 [ %1, %cond.true ], [ %div, %cond.false ]
  store i64 %cond, ptr @func_34___trans_tmp_29, align 8, !tbaa !11                store i64 %cond, ptr @func_34___trans_tmp_29, align 8, !tbaa !11
  %3 = load ptr, ptr @g_77, align 8, !tbaa !13                                    %3 = load ptr, ptr @g_77, align 8, !tbaa !13
  %4 = load i32, ptr %3, align 4, !tbaa !4                                        %4 = load i32, ptr %3, align 4, !tbaa !4
  %conv9 = trunc i32 %p_37 to i8                                                  %conv9 = trunc i32 %p_37 to i8
  %cmp.i = icmp eq i8 %conv9, 1                                                   %cmp.i = icmp eq i8 %conv9, 1
  %conv1.i = zext i1 %cmp.i to i32                                          <
  br i1 %cmp.i, label %safe_mod_func_uint8_t_u_u.exit, label %cond.false.i        br i1 %cmp.i, label %safe_mod_func_uint8_t_u_u.exit, label %cond.false.i

cond.false.i:                                     ; preds = %cond.end           cond.false.i:                                     ; preds = %cond.end
  %conv8 = trunc i32 %4 to i8                                                     %conv8 = trunc i32 %4 to i8
  %5 = urem i8 %conv8, %conv9                                                     %5 = urem i8 %conv8, %conv9
  %rem.i = zext i8 %5 to i32                                                |     %6 = icmp ugt i8 %5, 1
                                                                            >     %7 = zext i1 %6 to i32
  br label %safe_mod_func_uint8_t_u_u.exit                                        br label %safe_mod_func_uint8_t_u_u.exit

safe_mod_func_uint8_t_u_u.exit:                   ; preds = %cond.end, %co      safe_mod_func_uint8_t_u_u.exit:                   ; preds = %cond.end, %co
  %cond.i = phi i32 [ %rem.i, %cond.false.i ], [ %conv1.i, %cond.end ]      |     %cond.i = phi i32 [ %7, %cond.false.i ], [ 0, %cond.end ]
  %cmp10 = icmp ugt i32 %cond.i, 1                                          |     %or = or i32 %4, %cond.i
  %conv11 = zext i1 %cmp10 to i32                                           <
  %or = or i32 %4, %conv11                                                  <
  store i32 %or, ptr %3, align 4, !tbaa !4                                        store i32 %or, ptr %3, align 4, !tbaa !4
  %6 = load i32, ptr @safe_mul_func_uint32_t_u_u_ui1, align 4, !tbaa !4     |     %8 = load i32, ptr @safe_mul_func_uint32_t_u_u_ui1, align 4, !tbaa !4
  %idxprom = sext i32 %p_36.3 to i64                                              %idxprom = sext i32 %p_36.3 to i64
  %arrayidx = getelementptr inbounds [7 x i32], ptr @g_95, i64 0, i64 %idx        %arrayidx = getelementptr inbounds [7 x i32], ptr @g_95, i64 0, i64 %idx
  %7 = load i32, ptr %arrayidx, align 4, !tbaa !4                           |     %9 = load i32, ptr %arrayidx, align 4, !tbaa !4
  %dec = add nsw i32 %7, -1                                                 |     %dec = add nsw i32 %9, -1
  store i32 %dec, ptr %arrayidx, align 4, !tbaa !4                                store i32 %dec, ptr %arrayidx, align 4, !tbaa !4
  %mul = mul nsw i32 %6, %7                                                 |     %mul = mul nsw i32 %8, %9
  %conv12 = trunc i32 %mul to i8                                                  %conv12 = trunc i32 %mul to i8
  store i8 %conv12, ptr @g_185_1, align 1, !tbaa !8                               store i8 %conv12, ptr @g_185_1, align 1, !tbaa !8
  store i8 %conv12, ptr getelementptr inbounds (i8, ptr @g_255, i64 12), a        store i8 %conv12, ptr getelementptr inbounds (i8, ptr @g_255, i64 12), a
  %tobool16.not = icmp eq i32 %p_36.3, 0                                          %tobool16.not = icmp eq i32 %p_36.3, 0
  %tobool19.not = icmp eq i32 %p_37, 0                                            %tobool19.not = icmp eq i32 %p_37, 0
  %.47 = select i1 %tobool19.not, i32 0, i32 11                                   %.47 = select i1 %tobool19.not, i32 0, i32 11
  %8 = or i32 %p_36.3, %p_37                                                |     %10 = or i32 %p_36.3, %p_37
  %cond37 = icmp eq i32 %8, 0                                               |     %cond37 = icmp eq i32 %10, 0
  %cleanup.dest.slot.0 = select i1 %tobool16.not, i32 %.47, i32 9                 %cleanup.dest.slot.0 = select i1 %tobool16.not, i32 %.47, i32 9
  br i1 %cond37, label %cleanup.cont, label %cleanup22                            br i1 %cond37, label %cleanup.cont, label %cleanup22

@nikic

[nikic]

The 0 value looks correct to me. It's the result of (zext (i1 to i32) ugt 1) which is always false.

I can't repro using clang -O1 ~/Downloads/wrong0.i -emit-llvm -S -target s390x-unknown-linux-gnu -march=z15 and running the result through llvm-ub-aware-interpreter, so I think this is a backend bug.

[JonPsson1]

but the 0 comes from %cond.end ..?

To the left, %conv1.i is passed, which is the extended i1 value, and if it's 0, the br goes to %cond.false.i.

If %cmp.i is 1, br goes to %safe_mod_func_uint8_t_u_u.exit, and %conv1.i is 1.

Or am I missing something?

@nikic

[nikic]

@JonPsson1 I think you're missing that the meaning of %conv1.i changed. The transform replaced %conv1.i with zext(icmp(%conv1.i ugt 1) to i32).

I guess the fact that we keep the original name of the PHI instead of renaming it to %conv11 is somewhat confusing...

[JonPsson1]

@JonPsson1 I think you're missing that the meaning of %conv1.i changed. The transform replaced %conv1.i with zext(icmp(%conv1.i ugt 1) to i32).

I guess the fact that we keep the original name of the PHI instead of renaming it to %conv11 is somewhat confusing...

ok, thanks for the clarification - I'll keep looking then (in the backend, probably).