[clang] [Inliner] Improve attribute propagation to callsites when inlining. (PR #66036)

via cfe-commits cfe-commits at lists.llvm.org
Mon Sep 18 05:23:02 PDT 2023


goldsteinn wrote:

> > > It occurs to me that the current return attribute propagation is currently buggy for poison-generating attributes: https://llvm.godbolt.org/z/x8n18q9Mj
> > > In this case the argument to use() will now be poison as well, while before inlining only the return value was poison.
> > > This code needs to distinguish poison and UB generating attributes.
> > 
> > 
> > Good catch. I think this means basically `nonnull`, `noundef`, and `align` can only be propagated if there are no other uses in to-be-inlined function. That sound right or do you see any more robust way forward?
> 
> Limiting poison-generating attributes to one-use while keeping the rest of the logic (with guarantee-to-transfer) would be the simplest way to make the existing code correct. There are two additional ways to generalize:
> 
> * If the return is also noundef, we don't need one-use.
Is that right for `nonnull`?

I.e:
```
define noundef nonnull ptr @foo() {
   %b = call ptr @bar()
   call void @use(ptr %p) willreturn nounwind
   ret ptr %b
}
```
If we add `nonnull` to `@bar` during inlining it can still make `%p` poison for its use in `@use`. I get it will trigger proper UB at the return b.c the `noundef`, but is it okay to potentially trigger it early at the call to `@use`?


> * Just one-use is enough for poison-generating, we don't need guaranteed-to-transfer, BUT: we need to be careful about an implicit "use" in the call itself. That is, if the call we're transfering to is noundef and we only check one-use but not guaranteed-to-transfer, we would convert poison into UB at that point.

Oh I see, you mean something like:
```
define nonnull ptr @foo() {
   %b = call noundef ptr @bar()
   ret ptr %b
}
```

So it would convert `poison` ret to full UB if we transfer `nonnull`.
The otherway around too I guess i.e if we have:
```
define noundef ptr @foo() {
   %b = call nonull ptr @bar()
   ret ptr %b
}
```
We also need to be careful about transferring the `noundef`.

I think with one-use + guranteed to transfer (the latter is a precondition for transferring at all)
in the latter we would get UB either way, but we are moving to the point up a tiny bit.
I don't understand how this could ever be okay in the former case though? Isn't the former case
just straight up converting `poison` to UB which is a no-go?

https://github.com/llvm/llvm-project/pull/66036


More information about the cfe-commits mailing list