[PATCH] D103958: [WIP] Support MustControl conditional control attribute

nickdesaulniers added a comment. FWIW, LWN recently published summary of some of the recent discussions on LKML: https://lwn.net/SubscriberLink/860037/aca06acfafce7937/. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103958/new/ https://reviews.llv

[PATCH] D103958: [WIP] Support MustControl conditional control attribute

melver added a comment. In D103958#2811246 , @efriedma wrote: >> Defining the value used to establish a control dependency, e.g. the load >> later writes depend on (kernel only defines writes to be ctrl-dependently >> ordered). > > `[[mustcontrol]]` als

[PATCH] D103958: [WIP] Support MustControl conditional control attribute

efriedma added a comment. > Defining the value used to establish a control dependency, e.g. the load > later writes depend on (kernel only defines writes to be ctrl-dependently > ordered). `[[mustcontrol]]` also has this problem. At the LLVM IR level, if just want to split the load from the co

[PATCH] D103958: [WIP] Support MustControl conditional control attribute

melver updated this revision to Diff 351229. melver added a comment. As promised, some cleanups, docs, and updated test for the current version (no other major changes yet). While the identical-writes test is quite contrived, the currently failing switch test is a more realistic example. The exam

[PATCH] D103958: [WIP] Support MustControl conditional control attribute

melver added a comment. In D103958#2809353 , @efriedma wrote: > You could break `__builtin_load_with_control_dependency(x)` into something > like `__builtin_control_dependency(READ_ONCE(x))`. I don't think any > transforms will touch that in practice,

[PATCH] D103958: [WIP] Support MustControl conditional control attribute

efriedma added a comment. You could break `__builtin_load_with_control_dependency(x)` into something like `__builtin_control_dependency(READ_ONCE(x))`. I don't think any transforms will touch that in practice, even if it isn't theoretically sound. The rest of my suggestion still applies to th

[PATCH] D103958: [WIP] Support MustControl conditional control attribute

melver added a comment. In D103958#2809145 , @efriedma wrote: > In D103958#2808966 , @melver wrote: > >> In D103958#2808861 , @efriedma >> wrote: >> >>> I don't like usin

[PATCH] D103958: [WIP] Support MustControl conditional control attribute

efriedma added a comment. In D103958#2808966 , @melver wrote: > In D103958#2808861 , @efriedma > wrote: > >> I don't like using metadata like this. Dropping metadata should generally >> preserve the semantics o

[PATCH] D103958: [WIP] Support MustControl conditional control attribute

jdoerfert added subscribers: nhaehnle, rjmccall, arsenm. jdoerfert added a comment. This starts to sound an awful lot like `convergent` to me, basically: Do not change the control conditions of this call. Still unsure, maybe you can add a LangRef draft so we know what you try to do, or a nice ex

[PATCH] D103958: [WIP] Support MustControl conditional control attribute

melver added a comment. In D103958#2808967 , @jdoerfert wrote: >> Please bear with me, I'm updating examples and documentation. I didn't think >> anybody would look at this while [WIP]. :-) > > People try to help so you have early design feedback ;) Tha

[PATCH] D103958: [WIP] Support MustControl conditional control attribute

jdoerfert added a comment. > Please bear with me, I'm updating examples and documentation. I didn't think > anybody would look at this while [WIP]. :-) People try to help so you have early design feedback ;) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llv

[PATCH] D103958: [WIP] Support MustControl conditional control attribute

melver added a comment. In D103958#2808861 , @efriedma wrote: > I don't like using metadata like this. Dropping metadata should generally > preserve the semantics of the code. Anything better for this without introducing new instructions? Would an arg

[PATCH] D103958: [WIP] Support MustControl conditional control attribute

efriedma added a comment. In D103958#2808861 , @efriedma wrote: >> without resorting to inline assembly (which often results in poor codegen). > > Could you give an example of the resulting assembly with mustcontrol vs. this > patch? Err, I mean, the re

[PATCH] D103958: [WIP] Support MustControl conditional control attribute

efriedma added a comment. I don't like using metadata like this. Dropping metadata should generally preserve the semantics of the code. > without resorting to inline assembly (which often results in poor codegen). Could you give an example of the resulting assembly with mustcontrol vs. this p

[PATCH] D103958: [WIP] Support MustControl conditional control attribute

jdoerfert added a comment. In D103958#2808767 , @nickdesaulniers wrote: > The first talk from https://www.youtube.com/watch?v=FFjV9f_Ub9o > (https://github.com/ClangBuiltLinux/plumbers-2020-slides/blob/master/LPC_2020_--_Dependency_ordering.pdf) > migh

[PATCH] D103958: [WIP] Support MustControl conditional control attribute

nickdesaulniers added a comment. The first talk from https://www.youtube.com/watch?v=FFjV9f_Ub9o (https://github.com/ClangBuiltLinux/plumbers-2020-slides/blob/master/LPC_2020_--_Dependency_ordering.pdf) might be helpful to link to at some point from the commit message, for a little additional c

[PATCH] D103958: [WIP] Support MustControl conditional control attribute

jdoerfert added a comment. WIP here is fine, would help to include a test that shows/explains what problem is actually solved though. I don't understand form this patch alone. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103958/new/ https://revie

[PATCH] D103958: [WIP] Support MustControl conditional control attribute

melver added a comment. In D103958#2807811 , @lebedev.ri wrote: > This is missing langref changes, and a RFC to llvm-dev. We're not there yet, but will send something. Having real code helped me understand what out the myriad of options that were discu

[PATCH] D103958: [WIP] Support MustControl conditional control attribute

lebedev.ri added a comment. This is missing langref changes, and a RFC to llvm-dev. I'm rather skeptical of this. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103958/new/ https://reviews.llvm.org/D103958 __

[PATCH] D103958: [WIP] Support MustControl conditional control attribute

melver created this revision. Herald added subscribers: dexonsmith, jfb, hiraditya. Herald added a reviewer: aaron.ballman. melver requested review of this revision. Herald added subscribers: llvm-commits, cfe-commits, jdoerfert. Herald added projects: clang, LLVM. [ WIP, only high-level comments