[PATCH] D123991: [LangRef] Clarify load/store of non-byte-sized types

[Nikita Popov via Phabricator via llvm-commits]

nikic added a comment.

In D123991#3466373 <https://reviews.llvm.org/D123991#3466373>, @aqjune wrote:

> In D123991#3464364 <https://reviews.llvm.org/D123991#3464364>, @nlopes wrote:
>
>> Well, here we are discussing the semantics of LLVM IR. It's ok for the semantics of SDAG to be different, as long as it's a refinement of LLVM IR's.
>
> I think any valid compilation of LLVM IR should be considered here.
> There might be another compiler for LLVM IR that does not use SDAG internally. If padding is poison, it is valid for the other compiler to lower padding to non-zero bits, which will invalidate the transformations in various machines mentioned above.

Even with just the one compiler, if we take something like this:

  define i8 @test(ptr %p) {
    store i8 -1, ptr %p
    %v = load i1, ptr %p
    %r = zext i1 %v to i8
    ret i8 %r
  }

then with basic "padding is poison" semantics, this function must return `i8 1`, while the SDAG lowering would return `i8 -1` (https://llvm.godbolt.org/z/bMEYnaqah with an optimization barrier to prevent folding).

I think to satisfy all requirements while keeping the current lowering, we'd need to do something like this:

> **Loads**: When loading a non-byte-sized type, the result is poison if the padding bits do not have the value that a store of the same type would produce.



> **Stores**: When storing a non-byte-sized type, if the stored value is poison then the padding bits are set to poison, otherwise the padding bits are set to a target-specific value.

The convoluted wording is to make sure that dropping the load/store in the following example is legal:

  define void @src(ptr %p) {
    %v = load i1, ptr %p
    store i1 %v, ptr %p
    ret void
  }

Here, if `%p` currently has correct padding, then this is clearly a no-op. If it has incorrect padding, then the load returns poison, and then the store writes a poison byte, which can be refined to the current value at the location.

Though I am wondering if we wouldn't be better off with actually changing the backend lowering in the interest of having sane IR semantics. I.e. the backend would not assume any specific value for the padding bits of `load i1` and would have to introduce a masking operation. I don't think performance for these is particularly important, as frontends shouldn't be introducing such accesses anyway (at least clang and rustc will both use i8 as the in-memory type for i1).


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

https://reviews.llvm.org/D123991