Alex P. via llvm-dev
2020-Oct-25 20:24 UTC
[llvm-dev] "Unusual" linkage inhibits interprocedural constant propagation?
Hi Johannes, thanks for reply. I suspected that ipconstprop was not active in -O3 mode, but I did not know it was deprecated at all. However, either -O3 or -ipsccp behave the same way. BTW what other inter-procedural deductions should not apply for _odr linkage? As far as I understand, an _odr definition is quite similar to an extern definition semantically (well, according to C++'s definition of ODR rule)... On 25-Oct-20 12:08 PM, Johannes Doerfert wrote:> Hi Alex, > > this is a "bug", as far as I can tell. > > `_odr` linkage should allow inter-procedural propagation of constant > returns, > though prevent other inter-procedural deductions. This is why we are a bit > cautious with these things. > > I won't fix ipconstprop because we actually removed it but I will look > into an > extension of the Attributor to allow this. IPSCCP can probably also be > taught to > do this. > > ~ Johannes > > > On 10/23/20 10:40 PM, Alex P. via llvm-dev wrote: >> Dear LLVM developers and adopters! >> >> $ cat ipcp-1.ll >> define >> ;linkonce_odr >> dso_local i32 @f() noinline { >> ret i32 123 >> } >> define dso_local i32 @g() >> { >> %res = call i32 @f() >> ret i32 %res >> } >> $ opt-10 -S -ipconstprop ipcp-1.ll >> ; ModuleID = 'ipcp-1.ll' >> source_filename = "ipcp-1.ll" >> >> ; Function Attrs: noinline >> define dso_local i32 @f() #0 { >> ret i32 123 >> } >> >> define dso_local i32 @g() { >> %res = call i32 @f() >> ret i32 123 <========== note the result >> } >> >> attributes #0 = { noinline } >> >> BUT: >> >> $ cat ipcp-2.ll >> define >> linkonce_odr >> dso_local i32 @f() noinline { >> ret i32 123 >> } >> define dso_local i32 @g() >> { >> %res = call i32 @f() >> ret i32 %res >> } >> $ opt-10 -S -ipconstprop ipcp-2.ll >> ; ModuleID = 'ipcp-2.ll' >> source_filename = "ipcp-2.ll" >> >> ; Function Attrs: noinline >> define linkonce_odr dso_local i32 @f() #0 { >> ret i32 123 >> } >> >> define dso_local i32 @g() { >> %res = call i32 @f() >> ret i32 %res <========== note the (lack of) result >> } >> >> attributes #0 = { noinline } >> >> WHY? It this a bug? >> >> I observe the same behavior if I replace "-ipconstprop" with "-O3" or >> replace "linkonce_odr" with "available_externally", and if I use an >> equivalent testcase in C++ (compiled with the clang++ frontend). No >> problem with "external", "private" or "hidden" linkages. Also note >> that those "linkonce_odr"/"available_externally" do not inhibit, e.g., >> inlining (if I remove "noinline"), that is, as implied from the IR >> documentation. >> >> I am using LLVM version 10.0.0. >> >> This is a showstopper for my project (actually trying to use LLVM as >> an affordable static type inferer for a dynamically typed PL). >> >> Thanks for any help-- Alex <alex at webprise.net>
Johannes Doerfert via llvm-dev
2020-Oct-26 02:37 UTC
[llvm-dev] "Unusual" linkage inhibits interprocedural constant propagation?
IPConstProp was not in the default optimization pipeline for a long time and has been removed in LLVM11 (or shortly after). Both the Attributor nor IPSCCP perform the transformations IPConstProp did, though neither handles your case right now. The Attributor will not propagate information inter-procedurally, the relevant code in Attrinbutor.h (line 2190) describes the "problem" already: bool IsFnInterface = IRP.isFnInterfaceKind(); const Function *FnScope = IRP.getAnchorScope(); // TODO: Not all attributes require an exact definition. Find a way to // enable deduction for some but not all attributes in case the // definition might be changed at runtime, see also // http://lists.llvm.org/pipermail/llvm-dev/2018-February/121275.html. // TODO: We could always determine abstract attributes and if sufficient // information was found we could duplicate the functions that do not // have an exact definition. if (IsFnInterface && (!FnScope || !A.isFunctionIPOAmendable(*FnScope))) this->getState().indicatePessimisticFixpoint(); Note that we actually have code to do the duplication, though I need to push some fixes for this "deep wrapper" generation I have prepared locally. What you cannot do is, just as a simple example, derive readnone for a function, e.g., int f(int *a) { return 123; } While it clearly doesn't read or write any memory, a less optimized equivalent version could, e.g., the original code might have looked like this: int f(int *a) { return *a ? 123 : *a + 123; } which clearly reads memory. You can play this game with various other properties as well. However, the observed return value should never be different between equivalent versions of the function (up to non-deterministic choices) and I therefore think the return value can be propagated. If you want to get your hands dirty and teach the Attributor about it, that would be great. I would probably go with a method in AbstractAttribute that can be overwritten if the Attribute is OK with _odr linkage on function interface positions. The only time we overwrite would be in AAReturnedValues for now. Let me know what you think. ~ Johannes P.S. After I wrote this I wanted to make sure the information is correct. Turns out, AAReturnedValuesImpl::initialize does not call IRAttribute::initialize but instead basically duplicates the check. In llvm/lib/Transforms/IPO/AttributorAttributes.cpp line 821 it says if (!A.isFunctionIPOAmendable(*F)) indicatePessimisticFixpoint(); which is equivalent to the above because AAReturnedValues only exist for function interface positions anyway. So maybe we can for now just look for _odr linkage there. Or better, provide an argument to isFunctionIPOAmendable that determines if _odr is OK or not. On 10/25/20 3:24 PM, Alex P. wrote: > Hi Johannes, thanks for reply. I suspected that ipconstprop was not active in -O3 mode, but I did not know it was deprecated at all. However, either -O3 or -ipsccp behave the same way. > > BTW what other inter-procedural deductions should not apply for _odr linkage? As far as I understand, an _odr definition is quite similar to an extern definition semantically (well, according to C++'s definition of ODR rule)... > > On 25-Oct-20 12:08 PM, Johannes Doerfert wrote: >> Hi Alex, >> >> this is a "bug", as far as I can tell. >> >> `_odr` linkage should allow inter-procedural propagation of constant returns, >> though prevent other inter-procedural deductions. This is why we are a bit >> cautious with these things. >> >> I won't fix ipconstprop because we actually removed it but I will look into an >> extension of the Attributor to allow this. IPSCCP can probably also be taught to >> do this. >> >> ~ Johannes >> >> >> On 10/23/20 10:40 PM, Alex P. via llvm-dev wrote: >>> Dear LLVM developers and adopters! >>> >>> $ cat ipcp-1.ll >>> define >>> ;linkonce_odr >>> dso_local i32 @f() noinline { >>> ret i32 123 >>> } >>> define dso_local i32 @g() >>> { >>> %res = call i32 @f() >>> ret i32 %res >>> } >>> $ opt-10 -S -ipconstprop ipcp-1.ll >>> ; ModuleID = 'ipcp-1.ll' >>> source_filename = "ipcp-1.ll" >>> >>> ; Function Attrs: noinline >>> define dso_local i32 @f() #0 { >>> ret i32 123 >>> } >>> >>> define dso_local i32 @g() { >>> %res = call i32 @f() >>> ret i32 123 <========== note the result >>> } >>> >>> attributes #0 = { noinline } >>> >>> BUT: >>> >>> $ cat ipcp-2.ll >>> define >>> linkonce_odr >>> dso_local i32 @f() noinline { >>> ret i32 123 >>> } >>> define dso_local i32 @g() >>> { >>> %res = call i32 @f() >>> ret i32 %res >>> } >>> $ opt-10 -S -ipconstprop ipcp-2.ll >>> ; ModuleID = 'ipcp-2.ll' >>> source_filename = "ipcp-2.ll" >>> >>> ; Function Attrs: noinline >>> define linkonce_odr dso_local i32 @f() #0 { >>> ret i32 123 >>> } >>> >>> define dso_local i32 @g() { >>> %res = call i32 @f() >>> ret i32 %res <========== note the (lack of) result >>> } >>> >>> attributes #0 = { noinline } >>> >>> WHY? It this a bug? >>> >>> I observe the same behavior if I replace "-ipconstprop" with "-O3" or replace "linkonce_odr" with "available_externally", and if I use an equivalent testcase in C++ (compiled with the clang++ frontend). No problem with "external", "private" or "hidden" linkages. Also note that those "linkonce_odr"/"available_externally" do not inhibit, e.g., inlining (if I remove "noinline"), that is, as implied from the IR documentation. >>> >>> I am using LLVM version 10.0.0. >>> >>> This is a showstopper for my project (actually trying to use LLVM as an affordable static type inferer for a dynamically typed PL). >>> >>> Thanks for any help >
Alex P. via llvm-dev
2020-Oct-27 18:50 UTC
[llvm-dev] "Unusual" linkage inhibits interprocedural constant propagation?
Johannes, thank you for your explanations. Now I understand why the "bug" exists in the first place. BTW, according to your explanations, does this mean that we can/should treat the "available_externally" definitions exactly in the same way as just "external"? I understand that probably that is not specified precisely in the manual (and no standard like C++ covers the behavior in this case, unlike "_odr"). Should I now submit a bug report in order for us to proceed or you can do it yourself? On 25-Oct-20 9:37 PM, Johannes Doerfert wrote:> > IPConstProp was not in the default optimization pipeline for a long time > and has been removed in LLVM11 (or shortly after). > > Both the Attributor nor IPSCCP perform the transformations IPConstProp > did, though neither handles your case right now. The Attributor will not > propagate information inter-procedurally, the relevant code in > Attrinbutor.h (line 2190) describes the "problem" already: > > bool IsFnInterface = IRP.isFnInterfaceKind(); > const Function *FnScope = IRP.getAnchorScope(); > // TODO: Not all attributes require an exact definition. Find a > way to > // enable deduction for some but not all attributes in case the > // definition might be changed at runtime, see also > // > http://lists.llvm.org/pipermail/llvm-dev/2018-February/121275.html. > // TODO: We could always determine abstract attributes and if > sufficient > // information was found we could duplicate the functions > that do not > // have an exact definition. > if (IsFnInterface && (!FnScope || > !A.isFunctionIPOAmendable(*FnScope))) > this->getState().indicatePessimisticFixpoint(); > > Note that we actually have code to do the duplication, though I need to > push some fixes for this "deep wrapper" generation I have prepared > locally. > > What you cannot do is, just as a simple example, derive readnone for > a function, e.g., > int f(int *a) { return 123; } > > While it clearly doesn't read or write any memory, a less > optimized equivalent version could, e.g., the original code might have > looked like this: > > int f(int *a) { return *a ? 123 : *a + 123; } > > which clearly reads memory. You can play this game with various other > properties as well. However, the observed return value should never be > different between equivalent versions of the function (up to > non-deterministic choices) and I therefore think the return value can be > propagated. > > If you want to get your hands dirty and teach the Attributor about it, > that would be great. I would probably go with a method in > AbstractAttribute that can be overwritten if the Attribute is OK with > _odr linkage on function interface positions. The only time we overwrite > would be in AAReturnedValues for now. > > Let me know what you think. > > ~ Johannes > > P.S. After I wrote this I wanted to make sure the information is > correct. Turns out, AAReturnedValuesImpl::initialize does not call > IRAttribute::initialize but instead basically duplicates the check. In > llvm/lib/Transforms/IPO/AttributorAttributes.cpp line 821 > it says > if (!A.isFunctionIPOAmendable(*F)) > indicatePessimisticFixpoint(); > which is equivalent to the above because AAReturnedValues only exist for > function interface positions anyway. So maybe we can for now just look > for _odr linkage there. Or better, provide an argument to > isFunctionIPOAmendable that determines if _odr is OK or not. > > > > > On 10/25/20 3:24 PM, Alex P. wrote: > > Hi Johannes, thanks for reply. I suspected that ipconstprop was not > active in -O3 mode, but I did not know it was deprecated at all. > However, either -O3 or -ipsccp behave the same way. > > > > BTW what other inter-procedural deductions should not apply for _odr > linkage? As far as I understand, an _odr definition is quite similar to > an extern definition semantically (well, according to C++'s definition > of ODR rule)... > > > > On 25-Oct-20 12:08 PM, Johannes Doerfert wrote: > >> Hi Alex, > >> > >> this is a "bug", as far as I can tell. > >> > >> `_odr` linkage should allow inter-procedural propagation of constant > returns, > >> though prevent other inter-procedural deductions. This is why we are > a bit > >> cautious with these things. > >> > >> I won't fix ipconstprop because we actually removed it but I will > look into an > >> extension of the Attributor to allow this. IPSCCP can probably also > be taught to > >> do this. > >> > >> ~ Johannes > >> > >> > >> On 10/23/20 10:40 PM, Alex P. via llvm-dev wrote: > >>> Dear LLVM developers and adopters! > >>> > >>> $ cat ipcp-1.ll > >>> define > >>> ;linkonce_odr > >>> dso_local i32 @f() noinline { > >>> ret i32 123 > >>> } > >>> define dso_local i32 @g() > >>> { > >>> %res = call i32 @f() > >>> ret i32 %res > >>> } > >>> $ opt-10 -S -ipconstprop ipcp-1.ll > >>> ; ModuleID = 'ipcp-1.ll' > >>> source_filename = "ipcp-1.ll" > >>> > >>> ; Function Attrs: noinline > >>> define dso_local i32 @f() #0 { > >>> ret i32 123 > >>> } > >>> > >>> define dso_local i32 @g() { > >>> %res = call i32 @f() > >>> ret i32 123 <========== note the result > >>> } > >>> > >>> attributes #0 = { noinline } > >>> > >>> BUT: > >>> > >>> $ cat ipcp-2.ll > >>> define > >>> linkonce_odr > >>> dso_local i32 @f() noinline { > >>> ret i32 123 > >>> } > >>> define dso_local i32 @g() > >>> { > >>> %res = call i32 @f() > >>> ret i32 %res > >>> } > >>> $ opt-10 -S -ipconstprop ipcp-2.ll > >>> ; ModuleID = 'ipcp-2.ll' > >>> source_filename = "ipcp-2.ll" > >>> > >>> ; Function Attrs: noinline > >>> define linkonce_odr dso_local i32 @f() #0 { > >>> ret i32 123 > >>> } > >>> > >>> define dso_local i32 @g() { > >>> %res = call i32 @f() > >>> ret i32 %res <========== note the (lack of) result > >>> } > >>> > >>> attributes #0 = { noinline } > >>> > >>> WHY? It this a bug? > >>> > >>> I observe the same behavior if I replace "-ipconstprop" with "-O3" > or replace "linkonce_odr" with "available_externally", and if I use an > equivalent testcase in C++ (compiled with the clang++ frontend). No > problem with "external", "private" or "hidden" linkages. Also note that > those "linkonce_odr"/"available_externally" do not inhibit, e.g., > inlining (if I remove "noinline"), that is, as implied from the IR > documentation. > >>> > >>> I am using LLVM version 10.0.0. > >>> > >>> This is a showstopper for my project (actually trying to use LLVM > as an affordable static type inferer for a dynamically typed PL). > >>> > >>> Thanks for any help > > >-- Alex <alex at webprise.net>