[llvm-dev] sret read after unwind

Wed Jan 5 05:41:39 PST 2022

On Tue, Jan 4, 2022 at 6:15 PM Johannes Doerfert <johannesdoerfert at gmail.com>
wrote:

>
> On 1/4/22 10:57, Nikita Popov wrote:
> > On Tue, Jan 4, 2022 at 5:27 PM Johannes Doerfert <
> johannesdoerfert at gmail.com>
> > wrote:
> >
> >> On 1/4/22 03:39, Nikita Popov wrote:
> >>> On Mon, Jan 3, 2022 at 6:33 PM Johannes Doerfert <
> >> johannesdoerfert at gmail.com>
> >>> wrote:
> >>>
> >>>> I somewhat missed this thread and while I should maybe respond
> >>>> to a few of the other mails too I figured I start with a conceptual
> >>>> question I have reading this:
> >>>>
> >>>> Who and when is the attribute added? If it is implied by sret that's
> >>>> a good start. For the non-sret deduction it seems very specialized.
> >>>> I mean, now we have something for the unwind case but not a different
> >>>> "early exit" or if it is read/writeonly rather than readnone.
> >>>>
> >>> I'm mainly interested in frontend-annotated cases here, rather than
> >> deduced
> >>> ones. The primary use case there is adding it to sret arguments (and
> only
> >>> changing sret semantics would be "good enough" for me, I guess).
> However,
> >>> there is another frontend-annotated case I have my eyes on, which is
> move
> >>> arguments in rust. These could be modeled by a combination of
> >>> noreadonunwind and noreadonreturn to indicate that the value will not
> be
> >>> used after the call at all, regardless of how it exits. (This would be
> >> kind
> >>> of similar to a byval argument, just without the ABI implication that
> an
> >>> actual copy gets inserted.)
> >> OK. That's interesting. I'm not fluent enough in rust, can you
> >> elaborate what the semantics there would be, maybe an IR example?
> >>
> > To give a silly example, take these two functions in rust (
> > https://rust.godbolt.org/z/9cvefedsP):
> >
> > pub fn test1(mut s: String) {
> >      s = "foobar".to_string();
> > }
> > pub fn test2(s: &mut String) {
> >      *s = "foobar".to_string();
> > }
> >
> >  From an ABI perspective, these are basically the same. In both cases
> rust
> > will lower this to passing in a String*. However, because String is a
> > non-Copy type, any call "test(s)" will move the "s" variable, which means
> > that "s" cannot be used after the call anymore. For that reason, the
> store
> > "s =" would be definitely dead in the first example, and usually not dead
> > in the second example.
> >
> I see. Again just as an idea, what if we make the "overwriting stores"
> explicit instead of using attributes.
>
> ```
> s = "foobar".to_string();
> // other code
> virtual_memset(s, sizeof(s), 0);
> ret void
> ```
>
> Now DSE will do the work for us.
> It is not clear if we could do something similar for the other cases
> though.
>
> Whatever we do, I can see how this is information that is worth encoding.
>

Yeah, doing this kind of memset would encode the necessary information for
the return case -- I don't think it would be a good fit for the unwind
case, because it would require you to make all the unwind paths in the
function explicit. I think we already have this "virtual_memset" and it's
called llvm.lifetime.end -- just that nobody really understands its
semantics when it comes to non-alloca objects. In this case we'd have to
have the lifetime.start and lifetime.end in separate functions, which would
probably be all kinds of trouble :)

>> Spitballing: `byval(nocopy, %a)` might be worth thinking about
> >> given the short description.
> >>
> > Yeah, I guess that would work -- though I'd rather not mix ABI and
> > optimization attributes in that way...
> >
> >>> Note that as proposed, the noreadonunwind attribute would be the
> >> "writeonly
> >>> on unwind" combination (and noreadonreturn the "writeonly on return"
> >>> combination). I can see that there are conjugated "readonly on unwind"
> >> and
> >>> "readonly on return" attributes that could be defined here, but I can't
> >>> think of any circumstances under which these would actually be useful
> for
> >>> optimization purposes. How would the presence or absence of later
> writes
> >>> impact optimization in the current function?
> >> Just as an example, `readonly on unwind` allows you to do GVN/CSE
> >> from prior to the call to the "unwind path". Return then on the
> >> "return path". That is not inside the call but in the caller.
> >> Does that make sense?
> >
> > Let me check if I understood the idea right: We have a invoke with a
> > hypothetical "readonly on unwind" / "no write on unwind" attribute. In
> the
> > landing pad, there is a non-analyzable write and the pointer has
> previously
> > escaped, and then later there is a read from the argument. The
> > non-analyzable write blocks AA, while the "readonly on unwind" guarantee
> > could make a sufficiently smart AA see that this write cannot write into
> > the argument memory. Is that the idea? I feel like "readonly on unwind"
> > isn't the right way to model that situation, rather the argument could be
> > marked as invariant in the unwind path using one of our existing ways to
> > denote invariance.
> >
> > But I suspect I still didn't quite get what you have in mind here. An
> > example would help.
>
> Maybe I am confused but I thought something like this pseudo-code
> could be optimized, readonly_on_return is similar.
>
> ```
> int a = 42;
> invoke foo(/* readonly_on_unwind */ &a);
> lp:
>    return a; // a == 42
> cont:
>    return a; // a unknown
> ```
>

Okay, I guess there's a possible point of confusion here: The intention
here is that the attribute encodes a requirement on accesses *after* the
call. readonly_on_unwind would allow only reading "a" in "lp", but does not
prevent foo() from modifying "a" even if it ultimately unwinds. With that
in mind, I don't think the attribute would enable any additional
optimization here.

So maybe the proposed attribute is better named as "noreadafterunwind"
rather than "noreadonunwind".

Regards,
Nikita
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20220105/55c6b49f/attachment.html>