[llvm-dev] Inlining with different target features
Craig Topper via llvm-dev
llvm-dev at lists.llvm.org
Mon Aug 31 16:02:42 PDT 2020
That specific example, https://github.com/rust-lang/rust/issues/74320 seems
like it would be covered by the superset check right?
~Craig
On Mon, Aug 31, 2020 at 3:51 PM Thomas Lively via llvm-dev <
llvm-dev at lists.llvm.org> wrote:
> The difference is illustrated in this bug report:
> https://github.com/rust-lang/rust/issues/74320. Essentially, frontends
> like Rust want to allow users to opt-in to a feature by annotating a
> function that will use it. Your suggestion to require that all functions
> have the same feature set would prevent frontends from doing this sort of
> thing when targeting WebAsssembly. Which on one hand would be entirely
> reasonable, since annotating a single function with a feature misrepresents
> how WebAssembly features work, but on the other hand would make WebAssembly
> different from other targets, which I'd like to avoid.
>
> On Mon, Aug 31, 2020 at 2:13 PM David Blaikie <dblaikie at gmail.com> wrote:
>
>>
>>
>> On Mon, Aug 31, 2020 at 1:50 PM Thomas Lively via llvm-dev <
>> llvm-dev at lists.llvm.org> wrote:
>>
>>> David,
>>>
>>> That's right, WebAssembly does not have a way to conditionally use a
>>> feature or even do runtime feature testing right now. It's on our roadmap
>>> of things to design and standardize, but it is still a long way off.
>>>
>>> > Another direction would be to require the features to be specified
>>> consistently for all components of the build, I guess - if that's the net
>>> effect anyway. Would make portable libraries difficult, though - because
>>> they'd be linked into different things with different feature sets and that
>>> would violate the invariant.
>>>
>>> I agree this would be reasonable. We already require separate builds of
>>> a library for each feature set the library wants to support, so this
>>> wouldn't make that story any worse.
>>>
>>
>> So if that's already the case, what would change between that state and
>> what I'm suggesting?
>>
>>
>>> The only reason I wouldn't want to enforce this is because that would be
>>> another way targeting Wasm would be different from targeting other
>>> platforms for frontends.
>>>
>>> Eric,
>>>
>>> Updating all the features up front would indeed make the most sense. I
>>> haven't seen a way for backends to specify LLVM IR passes that should be
>>> run early, though. Is that possible, or would frontends have to add this
>>> extra pass when targeting Wasm?
>>>
>>>
>>>
>>>
>>> On Mon, Aug 31, 2020 at 1:40 PM Eric Christopher <echristo at gmail.com>
>>> wrote:
>>>
>>>> Hi Thomas,
>>>>
>>>> I'd prefer not to change areInlineCompatible because I think it reads
>>>> fairly closely what is expected here (also see x86 for how this is used for
>>>> subset inlining calculations). I think if you plan on updating all of the
>>>> features for the functions to match you might just want to do that
>>>> initially rather than try to update them piecemeal after or during
>>>> inlining.
>>>>
>>>> Thoughts?
>>>>
>>>> -eric
>>>>
>>>> On Mon, Aug 17, 2020 at 4:49 PM Thomas Lively via llvm-dev <
>>>> llvm-dev at lists.llvm.org> wrote:
>>>>
>>>>> Hi llvm-dev,
>>>>>
>>>>> I recently updated the WebAssembly TargetTransformInfo to allow
>>>>> functions with different target feature sets to be inlined into each other,
>>>>> but I ran into an issue I want to get the community's opinion on.
>>>>>
>>>>> Since WebAssembly modules have to be validated before they are run, it
>>>>> only makes sense to talk about WebAssembly features at module granularity
>>>>> rather than function granularity. The WebAssembly backend even runs a pass
>>>>> that applies the union of all used features to each function. That means
>>>>> that ideally inlining for the WebAssembly target would be able to disregard
>>>>> features entirely, since they will all be the same in the end.
>>>>>
>>>>> However, right now I have to be more conservative than that and only
>>>>> allow a callee to be inlined into a caller if the callee has a subset of
>>>>> the caller's features. Otherwise, a target intrinsic might end up being
>>>>> used in a function that does not have the necessary target features
>>>>> enabled, which would cause a validation failure.
>>>>>
>>>>> The best solution I can think of for this problem would be to allow
>>>>> targets to opt-in to having a caller's feature set updated to include the
>>>>> callee's feature set when the callee is inlined into the caller. This could
>>>>> be implemented via a new TTI hook, but a more general solution might be to
>>>>> change the return type of `areInlineCompatible` to allow targets to control
>>>>> this behavior on a case-by-case basis. Does this general direction sound
>>>>> ok, and if so, would it be better to add a new hook or add functionality to
>>>>> the existing one?
>>>>>
>>>>> Thanks,
>>>>>
>>>>> Thomas
>>>>> _______________________________________________
>>>>> LLVM Developers mailing list
>>>>> llvm-dev at lists.llvm.org
>>>>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>>>>
>>>> _______________________________________________
>>> LLVM Developers mailing list
>>> llvm-dev at lists.llvm.org
>>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>>
>> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20200831/d1c0e0a4/attachment.html>
More information about the llvm-dev
mailing list