Stack maps no longer experimental in 3.5

Hi all,

It is my understanding that now WebKit depends on the stack map functionality in production. Also, on the mailing lists we’ve seen lots of users using in this feature. Can we eliminate the experimental status for 3.5?

Off the top of my head, the changes needed are:

Anything else?

(also, we’ll have to put some stuff in http://llvm.org/docs/ReleaseNotes.html)

– Sean Silva

Hi all,

It is my understanding that now WebKit depends on the stack map functionality in production. Also, on the mailing lists we’ve seen lots of users using in this feature. Can we eliminate the experimental status for 3.5?

Correct. It is in production software and has multiple clients.

However, it may be worth doing another round of RFC before we pin down the intrinsic. There is an extension I would like to make, and I want to make sure we have covered the use cases of other clients that are just considering using the intrinsic but haven’t finalized their designs.

I planned to send another proposal next week. Based on that discussion we can determine whether to extend the existing intrinsics or add new intrinsics for new features.

Off the top of my head, the changes needed are:

Anything else?

Sounds good.

(also, we’ll have to put some stuff in http://llvm.org/docs/ReleaseNotes.html)

Right. I’m not in any particular hurry to make 3.5. Let’s see how the discussion goes next week.

-Andy

Hi all,

It is my understanding that now WebKit depends on the stack map
functionality in production. Also, on the mailing lists we've seen lots of
users using in this feature. Can we eliminate the experimental status for
3.5?

Correct. It is in production software and has multiple clients.

However, it may be worth doing another round of RFC before we pin down the
intrinsic. There is an extension I would like to make, and I want to make
sure we have covered the use cases of other clients that are just
considering using the intrinsic but haven’t finalized their designs.

Agreed.

I planned to send another proposal next week. Based on that discussion we
can determine whether to extend the existing intrinsics or add new
intrinsics for new features.

*nod*

Off the top of my head, the changes needed are:

- A read-through of StackMaps.rst to remove any mention of it being
experimental.
- Removing mention of it being experimental from
http://llvm.org/docs/LangRef.html#stack-map-intrinsics
- Removing the `.experimental` from the name.

Anything else?

Sounds good.

(also, we'll have to put some stuff in
http://llvm.org/docs/ReleaseNotes.html)

Right. I’m not in any particular hurry to make 3.5. Let’s see how the
discussion goes next week.

Sounds good to me. Thanks.

-eric

Hi all,

It is my understanding that now WebKit depends on the stack map
functionality in production. Also, on the mailing lists we've seen lots of
users using in this feature. Can we eliminate the experimental status for
3.5?

Correct. It is in production software and has multiple clients.

However, it may be worth doing another round of RFC before we pin down the
intrinsic. There is an extension I would like to make, and I want to make
sure we have covered the use cases of other clients that are just
considering using the intrinsic but haven’t finalized their designs.

I planned to send another proposal next week. Based on that discussion we
can determine whether to extend the existing intrinsics or add new
intrinsics for new features.

Cool. Sounds good.

-- Sean Silva

The only setback is to ensure that we synchronize the renaming with WebKit.

The WebKit->LLVM interface currently avoids revision-lock; you can take any recent revision of either and build a working browser engine. This is mostly true even when we change the stack map format because of versioning in the format. I’d rather keep it that way.

Is there a way to do this with intrinsics? I.e. is there a safe way for WebKit to query whether “llvm.patchpoint” is an available intrinsic, and then fallback to “llvm.experimental.patchpoint” if it’s not available?

-Fil

Keeping both names during a smallish time window should be sufficient, no?

Cheers,
Rafael

Not a blocker, but I'd really like to see someone who understands this code write a simple tutorial for it. The current documentation expects a lot of prior knowledge.

David

That would work. :slight_smile:

What about exposing C API a function to query for the presence of an intrinsic?

-Fil

That would work. :slight_smile:

What about exposing C API a function to query for the presence of an
intrinsic?

I don't know almost anything about how the C API works, but is it possible
to do something like `getHandleToIntrinsic(Module,
"llvm.experimental.patchpoint") == nullptr`? (I doubt that the function is
actually called "getHandleToIntrinsic", but you get the idea)

-- Sean Silva

> Off the top of my head, the changes needed are:
>
> - A read-through of StackMaps.rst to remove any mention of it being
experimental.
> - Removing mention of it being experimental from
http://llvm.org/docs/LangRef.html#stack-map-intrinsics
> - Removing the `.experimental` from the name.
>
> Anything else?

Not a blocker, but I'd really like to see someone who understands this
code write a simple tutorial for it.

A tutorial for this feature would mostly overlap with an MCJIT tutorial. I
believe it would be better to have a good MCJIT tutorial and then a
separate page describing the key places where the plain MCJIT tutorial
would need to be modified to utilize the stack map functionality.

The current documentation expects a lot of prior knowledge.

Well, making any use of the feature basically requires the user to patch
machine-code, and IMO that's the biggest hurdle (teaching machine-code
patching certainly isn't germane for a document targeted at a specific LLVM
feature).

For someone already familiar at a machine-code level with their
architecture of interest, then the feature is conceptually extremely
simple: it just gives you some guarantees about the structure of the
machine code at particular addresses and also gives you information about
where IR-level values are stored at run-time. More precisely:
- Both llvm.experimental.stackmap and llvm.experimental.patchpoint can
record the run-time location of IR-level values.
- Both can reserve space (a "shadow") in the machine code, but with
different guarantees: llvm.experimental.patchpoint makes sure that the
reserved region doesn't contain any other code besides a call of your
choosing (and nops if needed), while llvm.experimental.stackmap just
ensures (with nops if needed) that if you overwrite that space that you
will not be writing outside the function itself (this is meant for
destructively patching the following machine code).
These bullets are basically just a rehashing of:
http://llvm.org/docs/StackMaps.html#intrinsics

For someone comfortable patching machine code, the only difficulty I can
imagine are just janitorial details (such as setting up MCJIT or processing
the stack map data structure) to get snagged on, which can be covered in a
targeted (not tutorial) way.

-- Sean Silva

That would work. :slight_smile:

What about exposing C API a function to query for the presence of an intrinsic?

It seems with hindsight that the "experimental" prefix has turned out to be a waste of time.

At least without the prefix there was a good chance this churn could be avoided as long as the original review was sound, whereas the prefix has necessitated churn.

I suggest performing a configure-time check in your build system to retain backward/forward compatibility instead of attempting to specify C API for instruction introspection(!).

Alp.

That would work. :slight_smile:

What about exposing C API a function to query for the presence of an intrinsic?

It seems with hindsight that the “experimental” prefix has turned out to
be a waste of time.

It’s only a waste of time if you think that run-time feature detection is a bad idea. I believe that run-time feature detection is a good idea even if we didn’t need it in this particular case.

At least without the prefix there was a good chance this churn could be
avoided as long as the original review was sound, whereas the prefix has
necessitated churn.

The intrinsic changed a fair bit since the original review. Both its signature and the stackmap format changed.

I suggest performing a configure-time check in your build system to
retain backward/forward compatibility instead of attempting to specify C
API for instruction introspection(!).

Run-time checks are more robust than configure-time checks. LLVM is a moving target and new intrinsics are added all the time, and it shouldn’t be necessary to recompile all users of LLVM just to get them to recognize that the LLVM to which they got linked has the intrinsic that they already knew about.

-Filip

That would work. :slight_smile:

What about exposing C API a function to query for the presence of an
intrinsic?

It seems with hindsight that the "experimental" prefix has turned out to
be a waste of time.

It's only a waste of time if you think that run-time feature detection is a
bad idea. I believe that run-time feature detection is a good idea even if
we didn't need it in this particular case.

At least without the prefix there was a good chance this churn could be
avoided as long as the original review was sound, whereas the prefix has
necessitated churn.

The intrinsic changed a fair bit since the original review. Both its
signature and the stackmap format changed.

I suggest performing a configure-time check in your build system to
retain backward/forward compatibility instead of attempting to specify C
API for instruction introspection(!).

Run-time checks are more robust than configure-time checks. LLVM is a
moving target and new intrinsics are added all the time, and it shouldn't be
necessary to recompile all users of LLVM just to get them to recognize that
the LLVM to which they got linked has the intrinsic that they already knew
about.

Agreed. Sean's idea might work? If not, then I'm down for coming up
with something.

-eric

Couldn’t we just use the auto upgrader for this?
-Juergen

Only on bitcode that's already written, but I was assuming that Filip
had code that used the textual name of the intrinsic as a lookup to
get it which autoupgrade I didn't think would fix? (I could be wrong
here).

-eric

Dang. Yes, you are right. I forgot that we only run the autoupgrader on files that we read in and not on IR that is generated with C API calls.
-Juergen

That would work. :slight_smile:

What about exposing C API a function to query for the presence of an
intrinsic?

It seems with hindsight that the “experimental” prefix has turned out to
be a waste of time.

It’s only a waste of time if you think that run-time feature detection is a
bad idea. I believe that run-time feature detection is a good idea even if
we didn’t need it in this particular case.

At least without the prefix there was a good chance this churn could be
avoided as long as the original review was sound, whereas the prefix has
necessitated churn.

The intrinsic changed a fair bit since the original review. Both its
signature and the stackmap format changed.

I suggest performing a configure-time check in your build system to
retain backward/forward compatibility instead of attempting to specify C
API for instruction introspection(!).

Run-time checks are more robust than configure-time checks. LLVM is a
moving target and new intrinsics are added all the time, and it shouldn’t be
necessary to recompile all users of LLVM just to get them to recognize that
the LLVM to which they got linked has the intrinsic that they already knew
about.

Agreed. Sean’s idea might work? If not, then I’m down for coming up
with something.

This is clearly good for JITs and doesn’t have to be anything fancy. Anyone want to file a PR for it?

-Andy