wrong code at -O1 on x86_64-linux-gnu

[zhendongsu]

It appears to be a regression from 14.0.0, and affects 15.* and later.

Compiler Explorer: https://godbolt.org/z/57cnEr81r

[546] % clangtk -v
clang version 19.0.0git (https://github.com/llvm/llvm-project.git 07a566793b2f94d0de6b95b7e6d1146b0d7ffe49)
Target: x86_64-unknown-linux-gnu
Thread model: posix
InstalledDir: /local/suz-local/software/local/clang-trunk/bin
Found candidate GCC installation: /usr/lib/gcc/x86_64-linux-gnu/10
Found candidate GCC installation: /usr/lib/gcc/x86_64-linux-gnu/11
Found candidate GCC installation: /usr/lib/gcc/x86_64-linux-gnu/9
Selected GCC installation: /usr/lib/gcc/x86_64-linux-gnu/11
Candidate multilib: .;@m64
Selected multilib: .;@m64
[547] % 
[547] % clangtk -O1 small.c
[548] % ./a.out
Aborted
[549] % cat small.c
struct a {
  int b;
  int c;
  int d;
  int e;
  int f;
} g, h = {1,0,0,0,0}, *i;
void j(struct a k) {
  struct a *l = &g;
  if (k.b)
    l = i = &k;
  if (l != &k)
    __builtin_abort();
}
int main() {
  j(h);
  return 0;
}

[antoniofrighetto]

Looks like a wrong optimization performed by InstSimplify while attempting to inline j into main 🤔

[dtcxzyw]

Reduced test case: https://godbolt.org/z/racWKbrbr

%struct.a = type { i32, i32 }

@h = dso_local global %struct.a { i32 1, i32 0 }, align 8

define i1 @callee(ptr byval(%struct.a) align 8 %k) {
  %cmp = icmp eq ptr @h, %k
  ret i1 %cmp
}

define i1 @caller() {
  %ret = call i1 @callee(ptr byval(%struct.a) align 8 @h)
  ret i1 %ret
}

I am not sure if it is an UB. cc @shafik @frederick-vs-ja

[antoniofrighetto]

I believe that accessing the pointer (i) when the object it points to gets out of scope is UB, which doesn't happen in the original snippet. I think there may be still a logic bug while instructions get simplified during inlining. This is how the basic blocks are laid out during CloneBlock:

1:                                                ; No predecessors!
  %2 = load i32, ptr %h, align 8
  %.not.i = icmp eq i32 %2, 0
  br i1 %.not, label %4, label %3

3:                                                ; No predecessors!
  store ptr %h, ptr @i, align 8
  br label %4

4:                                                ; No predecessors!
  %.0.i = phi ptr [ %0, %3 ], [ @g, %1 ]
  %5 = icmp eq ptr %.0.i, %h
  br i1 %.not1, label %6, label %5

%0 has been remapped to %h everywhere but in the phi-node, as per:

llvm-project/llvm/lib/Transforms/Utils/CloneFunction.cpp

Lines 536 to 541 in ffd08c7

	// Eagerly remap operands to the newly cloned instruction, except for PHI
	// nodes for which we defer processing until we update the CFG. Also defer
	// debug intrinsic processing because they may contain use-before-defs.
	if (!isa<PHINode>(NewInst) && !isa<DbgVariableIntrinsic>(NewInst)) {
	RemapInstruction(NewInst, VMap,
	ModuleLevelChanges ? RF_None : RF_NoModuleLevelChanges);

The idiosyncrasy stems from the fact that %5 = icmp eq ptr %.0.i, %h is (correctly) simplified to i1 false, except that it shouldn't since %0 was not remapped in the PN.

[jdoerfert]

@antoniofrighetto described the problem already.

Funnily, 7 years ago this popped up for the first time:
9282f1a

Basically, we reason about / simplify instructions based on partially state data.
This cannot work.
The commit above was just a stop gap as it identified some situations when it went wrong (namely we ended up with a simplified instructions from another function).
Following the same logic, you could prevent the folder to come up with constants, but then it's questionable why we have it in the first place.

Alternatives include:

1. Remove the simplification, doing it at this stage, with half-broken IR, is simply not a good idea.
2. Move the simplification later, after the control flow and PHIs have been remapped.
3. Somehow make SimplifyInst stop at/before PHI nodes.

I would measure the impact of 1) first, that seems the cleanest solution.
You can prototype 3) fairly easily, but it's a whack-a-mole situation waiting to happen again (IMHO).
2) Is reasonable and maybe not as intrusive after all, haven't looked at it in much detail.