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

via cfe-commits cfe-commits at lists.llvm.org
Tue Sep 19 15:55:07 PDT 2023


goldsteinn wrote:

> > ```
> > 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`?
> 
> Yes, this is fine, because the UB will be hit anyway. We can move it backwards, as long as it's guaranteed to execute.
> 
> > > * 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?
> 
> You are right, the former case isn't correct even with one-use to guaranteed-to-transfer. That case would only work if we also had noundef on the foo return value.

@nikic, on the former case, could we just strip `noundef` from the ret attributes?
Just a hunch, but think `nonnull`, `align`,... are generally more useful.

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


More information about the cfe-commits mailing list