[all-commits] [llvm/llvm-project] 187e02: [CodeGenPrepare] Check types when unmerging GEPs a...

[Maurice Heumann via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: 187e02fa2deda9a01563c146d7daabdaf7e5108d
      https://github.com/llvm/llvm-project/commit/187e02fa2deda9a01563c146d7daabdaf7e5108d
  Author: Maurice Heumann <MauriceHeumann at gmail.com>
  Date:   2023-10-13 (Fri, 13 Oct 2023)

  Changed paths:
    M llvm/lib/CodeGen/CodeGenPrepare.cpp
    A llvm/test/CodeGen/X86/indirect-br-gep-unmerge.ll

  Log Message:
  -----------
  [CodeGenPrepare] Check types when unmerging GEPs across indirect branches (#68587)

The optimization in CodeGenPrepare, where GEPs are unmerged across
indirect branches must respect the types of both GEPs and their sizes
when adjusting the indices.

The sample here shows the bug:

https://godbolt.org/z/8e9o5sYPP

The value `%elementValuePtr` addresses the second field of the
`%struct.Blub`. It is therefore a GEP with index 1 and type i8.
The value `%nextArrayElement` addresses the next array element. It is
therefore a GEP with index 1 and type `%struct.Blub`.

Both values point to completely different addresses, even if the indices
are the same, due to the types being different.
However, after CodeGenPrepare has run, `%nextArrayElement` is a bitcast
from `%elementValuePtr`, meaning both were treated as equal.

The cause for this is that the unmerging optimization does not take
types into consideration.
It sees both GEPs have `%currentArrayElement` as source operand and
therefore tries to rewrite `%nextArrayElement` in terms of
`%elementValuePtr`.
It changes the index to the difference of the two GEPs. As both indices
are `1`, the difference is `0`. As the indices are `0` the GEP is later
replaced with a simple bitcast in CodeGenPrepare.

Before adjusting the indices, the types of the GEPs would have to be
aligned and the indices scaled accordingly for the optimization to be
correct.
Due to the size of the struct being `16` and the `%elementValuePtr`
pointing to offset `1`, the correct index for the unmerged
`%nextArrayElement` would be 15.

I assume this bug emerged from the opaque pointer change as GEPs like
`%elementValuePtr` that access the struct field based of type i8 did not
naturally occur before.

In light of future migration to ptradd, simply not performing the
optimization if the types mismatch should be sufficient.