[PATCH] D154051: [LangRef] Always allow getelementptr inbounds with zero offset
Nikita Popov via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Jun 30 01:58:26 PDT 2023
nikic added a comment.
In D154051#4463038 <https://reviews.llvm.org/D154051#4463038>, @nlopes wrote:
> I've implemented this semantics in Alive2 and run it over LLVM's test suite.
Thanks for testing!
> There are only 2 regressions, and they seem ok:
>
> + LLVM :: Transforms/InstCombine/gep-inbounds-null.ll
> define i1 @test_eq(ptr %base, i64 %idx) {
> %gep = gep inbounds ptr %base, 1 x i64 %idx
> %cnd = icmp eq ptr %gep, null
> ret i1 %cnd
> }
> =>
> define i1 @test_eq(ptr %base, i64 %idx) {
> %cnd = icmp eq ptr %base, null ; wrong since %base can be OOB
> ret i1 %cnd
> }
Hm, I don't think I understand why that transform is not valid anymore. Going through the different cases here:
- `%base == null`, `%idx == 0` -> `%cnd == true`.
- `%base == null`, `%idx != 0` -> `%cnd == poison`.
- `%base != null`, `%idx == 0` -> `%cnd == false` (by assumption).
- `%base != null`, `%idx != 0` -> `%cnd == false`. This is because it can only be true for `%idx = -ptrtoint(%base)`, which cannot be inbounds in the default address space, as no allocated object can start at address zero.
So I think reducing this to `%base == null` remains correct?
> + LLVM :: Transforms/InstSimplify/gep.ll
> define ptr @undef_inbounds_var_idx(i64 %idx) {
> %el = gep inbounds ptr undef, 8 x i64 %idx
> ret ptr %el
> }
> =>
> define ptr @undef_inbounds_var_idx(i64 %idx) {
> ret ptr poison ; only ok if %idx != 0
> }
Agree that this is no longer correct, we need to return `undef` instead. That's okay as we can still return `poison` for a `poison` base.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D154051/new/
https://reviews.llvm.org/D154051
More information about the llvm-commits
mailing list