On Fri, Dec 9, 2016 at 1:30 PM, Friedman, Eli via llvm-dev < llvm-dev at lists.llvm.org> wrote:> On 12/9/2016 8:45 AM, Reid Kleckner wrote: > > On Thu, Dec 8, 2016 at 5:37 PM, Mehdi Amini <mehdi.amini at apple.com> wrote: > >> So IIUC basically the *only* reason for this IR change is that we don’t >> want to pattern match in debug build? >> I don't understand right now why we wouldn’t want to do this? >> > > If we need to pattern match such a basic construct, it suggests to me that > we have the wrong representation, and we should instead make our > representation more accurately model reality. To me, it feels like this > representation allows several good things to just "fall out" without any > additional work, and that suggests it's a good representation. > > > Mmm... maybe. The part I really don't like is the implied store: there > are a lot of transformations which specifically reason about store > instructions, and they would all need to be fixed to specifically deal with > "alloca with an argument" so it doesn't block optimizations. Every feature > we add to the IR makes it more difficult to write IR transformations, and > this really doesn't seem to carry its own weight. > > A couple of other thoughts I had: > 1. We could use metadata, e.g. "%px = alloca i32, align 4 !argument !i32 > %x", followed by a store. There's still a little pattern-matching involved > because the metadata is only a hint, but the intent in the IR is more > explicit. > 2. We could change the way clang generates function definitions: instead > of "define void @f(i32 %x)", generate "define void @f(i32* byval %px)" if > the value is going to end up on the stack. This should work without any > changes to the IR. This is a bad idea if we're trying to run optimizations > because it obscures data flow, but it seems reasonable at -O0. >IMO, the LLVM function definitions should be a straightforward transformation from the C function signatures, and clang should stop mangling the function signatures with its own intimate knowledge of the calling convention rules. Instead, clang could emit (still ABI-specific!) annotations on the arguments that communicates the *properties* of the source-language types which affect the ABI. LLVM, then, would use that information to implement the complete calling convention. No counting of numbers of registers in Clang to put "inreg" markers in the right places, or byval, or any of that sort of stuff it has to do today. For example, on x86-64 SysV ABI, a complex classification algorithm is run over the fields in an aggregate, to classify each eightbyte quantity into one of several classes (MEMORY, INTEGER, SSE, etc). That must run on the clang side, using source types, but it could send the direct result of the classification to llvm, rather than using it to mangle the prototype of the function in a secret undocumented handshake using the few seemingly-but-not-really-generic attributes that are available now. For that reason, I really don't like the second option you mention above, as it requires clang to know more about exactly what the calling convention decisions made by llvm are going to be. Actually, I think byval should be deleted entirely. If you want to pass by value...then, pass by value... -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20161209/83a4b15a/attachment.html>
----- Original Message -----> From: "James Y Knight via llvm-dev" <llvm-dev at lists.llvm.org> > To: "Eli Friedman" <efriedma at codeaurora.org> > Cc: "llvm-dev" <llvm-dev at lists.llvm.org> > Sent: Friday, December 9, 2016 6:04:18 PM > Subject: Re: [llvm-dev] RFC: Adding argument allocas> On Fri, Dec 9, 2016 at 1:30 PM, Friedman, Eli via llvm-dev < > llvm-dev at lists.llvm.org > wrote:> > On 12/9/2016 8:45 AM, Reid Kleckner wrote: >> > > On Thu, Dec 8, 2016 at 5:37 PM, Mehdi Amini < > > > mehdi.amini at apple.com > > > > > > > wrote: > > >> > > > So IIUC basically the *only* reason for this IR change is that > > > > we > > > > don’t want to pattern match in debug build? > > > > > > > > > > I don't understand right now why we wouldn’t want to do this? > > > > > > > > > If we need to pattern match such a basic construct, it suggests > > > to > > > me > > > that we have the wrong representation, and we should instead make > > > our representation more accurately model reality. To me, it feels > > > like this representation allows several good things to just "fall > > > out" without any additional work, and that suggests it's a good > > > representation. > > > > > Mmm... maybe. The part I really don't like is the implied store: > > there are a lot of transformations which specifically reason about > > store instructions, and they would all need to be fixed to > > specifically deal with "alloca with an argument" so it doesn't > > block > > optimizations. Every feature we add to the IR makes it more > > difficult to write IR transformations, and this really doesn't seem > > to carry its own weight. >> > A couple of other thoughts I had: > > > 1. We could use metadata, e.g. "%px = alloca i32, align 4 !argument > > !i32 %x", followed by a store. There's still a little > > pattern-matching involved because the metadata is only a hint, but > > the intent in the IR is more explicit. > > > 2. We could change the way clang generates function definitions: > > instead of "define void @f(i32 %x)", generate "define void @f(i32* > > byval %px)" if the value is going to end up on the stack. This > > should work without any changes to the IR. This is a bad idea if > > we're trying to run optimizations because it obscures data flow, > > but > > it seems reasonable at -O0. > > IMO, the LLVM function definitions should be a straightforward > transformation from the C function signatures, and clang should stop > mangling the function signatures with its own intimate knowledge of > the calling convention rules.> Instead, clang could emit (still ABI-specific!) annotations on the > arguments that communicates the *properties* of the source-language > types which affect the ABI. LLVM, then, would use that information > to implement the complete calling convention. No counting of numbers > of registers in Clang to put "inreg" markers in the right places, or > byval, or any of that sort of stuff it has to do today.> For example, on x86-64 SysV ABI, a complex classification algorithm > is run over the fields in an aggregate, to classify each eightbyte > quantity into one of several classes (MEMORY, INTEGER, SSE, etc). > That must run on the clang side, using source types, but it could > send the direct result of the classification to llvm, rather than > using it to mangle the prototype of the function in a secret > undocumented handshake using the few > seemingly-but-not-really-generic attributes that are available now.If we implement a system which worked like this, that would be spectacular! :-) The fact that so much of our ABI knowledge is locked up in Clang is fragile, causes difficulties with implementing other frontends, etc. -Hal> For that reason, I really don't like the second option you mention > above, as it requires clang to know more about exactly what the > calling convention decisions made by llvm are going to be.> Actually, I think byval should be deleted entirely. If you want to > pass by value...then, pass by value... > _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev-- Hal Finkel Lead, Compiler Technology and Programming Languages Leadership Computing Facility Argonne National Laboratory -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20161210/37805fcc/attachment.html>
On Fri, Dec 9, 2016 at 4:04 PM, James Y Knight <jyknight at google.com> wrote:> IMO, the LLVM function definitions should be a straightforward > transformation from the C function signatures, and clang should stop > mangling the function signatures with its own intimate knowledge of the > calling convention rules. > > Instead, clang could emit (still ABI-specific!) annotations on the > arguments that communicates the *properties* of the source-language types > which affect the ABI. LLVM, then, would use that information to implement > the complete calling convention. No counting of numbers of registers in > Clang to put "inreg" markers in the right places, or byval, or any of that > sort of stuff it has to do today. >Clang's ABI lowering is part of why I don't want to pattern match alloca+store in LLVM. Today here is how we pass 'struct P { double x, y; }' on x86_64-linux: define void @f(double %p.coerce0, double %p.coerce1) #0 { entry: %p = alloca %struct.P, align 8 %0 = bitcast %struct.P* %p to { double, double }* %1 = getelementptr inbounds { double, double }, { double, double }* %0, i32 0, i32 0 store double %p.coerce0, double* %1, align 8 %2 = getelementptr inbounds { double, double }, { double, double }* %0, i32 0, i32 1 store double %p.coerce1, double* %2, align 8 ret void } My proposal doesn't make this any better so long as we split these things in the frontend, so let's assume we go the direction you propose, so that we receive just one %struct.P value. I'm pretty sure full FCA stores are non-canonical, so after optimizations, this is what we'd have to pattern match in the backend: define void @f(%struct.P %p) #0 { %p.addr = alloca %struct.P, align 8 %x.addr = getelementptr inbounds %struct.P, %struct.P* %0, i32 0, i32 0 %x = extractvalue %struct.P %p, 0 store double %x, double* %x.addr, align 8 %y.addr = getelementptr inbounds %struct.P, %struct.P* %0, i32 0, i32 1 %y = extractvalue %struct.P %p, 1 store double %y, double* %y.addr, align 8 ret void } This is doable, but I don't like it. It's not clear if we have consensus to stop splitting structs in clang, but that is the direction I'd like to go in the future. It would also be a bit of a bitcode ABI break, similar to the way the byval change. -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20161212/01d44094/attachment.html>
----- Original Message -----> From: "Reid Kleckner" <rnk at google.com> > To: "James Y Knight" <jyknight at google.com>, "Hal Finkel" > <hfinkel at anl.gov> > Cc: "Eli Friedman" <efriedma at codeaurora.org>, "llvm-dev" > <llvm-dev at lists.llvm.org> > Sent: Monday, December 12, 2016 3:56:47 PM > Subject: Re: [llvm-dev] RFC: Adding argument allocas> On Fri, Dec 9, 2016 at 4:04 PM, James Y Knight < jyknight at google.com > > wrote:> > IMO, the LLVM function definitions should be a straightforward > > transformation from the C function signatures, and clang should > > stop > > mangling the function signatures with its own intimate knowledge of > > the calling convention rules. >> > Instead, clang could emit (still ABI-specific!) annotations on the > > arguments that communicates the *properties* of the source-language > > types which affect the ABI. LLVM, then, would use that information > > to implement the complete calling convention. No counting of > > numbers > > of registers in Clang to put "inreg" markers in the right places, > > or > > byval, or any of that sort of stuff it has to do today. > > Clang's ABI lowering is part of why I don't want to pattern match > alloca+store in LLVM. Today here is how we pass 'struct P { double > x, y; }' on x86_64-linux:> define void @f(double %p.coerce0, double %p.coerce1) #0 { > entry: > %p = alloca %struct.P, align 8 > %0 = bitcast %struct.P* %p to { double, double }* > %1 = getelementptr inbounds { double, double }, { double, double }* > %0, i32 0, i32 0 > store double %p.coerce0, double* %1, align 8 > %2 = getelementptr inbounds { double, double }, { double, double }* > %0, i32 0, i32 1 > store double %p.coerce1, double* %2, align 8 > ret void > }> My proposal doesn't make this any better so long as we split these > things in the frontend, so let's assume we go the direction you > propose, so that we receive just one %struct.P value. I'm pretty > sure full FCA stores are non-canonical, so after optimizations, this > is what we'd have to pattern match in the backend:> define void @f(%struct.P %p) #0 { > %p.addr = alloca %struct.P, align 8 > %x.addr = getelementptr inbounds %struct.P, %struct.P* %0, i32 0, i32 > 0 > %x = extractvalue %struct.P %p, 0 > store double %x, double* %x.addr, align 8 > %y.addr = getelementptr inbounds %struct.P, %struct.P* %0, i32 0, i32 > 1 > %y = extractvalue %struct.P %p, 1> store double %y, double* %y.addr, align 8 > ret void > }> This is doable, but I don't like it.> It's not clear if we have consensus to stop splitting structs in > clang, but that is the direction I'd like to go in the future. It > would also be a bit of a bitcode ABI break, similar to the way the > byval change.We could always predicate the new behavior on some attribute, etc., and so I suspect that we could maintain backward compatibility for FCA arguments if we'd like. -Hal -- Hal Finkel Lead, Compiler Technology and Programming Languages Leadership Computing Facility Argonne National Laboratory -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20161212/397e059a/attachment.html>