Testcase: dzur:~/tmp> cat bar.c int t; extern inline void __attribute__((__gnu_inline__)) memcpy(void *a, const void *restrict x, unsigned long b) { __builtin_memcpy((a), x, b); } int main() { memcpy((void *)(&t), &t, 1); return 0; } things that also fail are prefetch, etc: extern inline void __attribute__((__gnu_inline__)) prefetch(const void *restrict x) { __builtin_prefetch ((x), 0, 1); } int t; int main() { prefetch((void *)(&t)); return 0; } both extern inline and __gnu_inline__ appear to be necessary here.
this testcase also fails: __inline __attribute__((always_inline)) int abs(int j) { return __builtin_abs(j); } int main() { return abs(0); } The issue is that we believe the function is trivially recursive: 1. we call clang::CodeGen::CodeGenModule::EmitGlobalDefinition 2. this calls clang::CodeGen::CodeGenModule::shouldEmitFunction 3. this calls clang::CodeGen::CodeGenModule::isTriviallyRecursive isTriviallyRecursive looks for a call to __builtin_abs (or __builtin_prefetch in the original example). Perhaps we should only bother running isTriviallyRecursive if the function is *not* marked always_inline?
What's the proper handling for defining a built in (not sure what the right term is - intrinsic?) function like abs, memcpy, etc? I imagine we should be either in one of two modes: 1) assume these things already exist and reject attempts to define them 2) assume they don't exist and that this TU is defining them - in which case we shouldn't be trying to treat them as the builtin form we would provide in (1) no?
(In reply to comment #3) > What's the proper handling for defining a built in (not sure what the right > term is - intrinsic?) function like abs, memcpy, etc? > > I imagine we should be either in one of two modes: > > 1) assume these things already exist and reject attempts to define them What happens if the call is coming from inside the house: *you* are the implementation and are trying to provide a definition. > 2) assume they don't exist and that this TU is defining them - in which case > we shouldn't be trying to treat them as the builtin form we would provide in > (1) > > no? What if your definition needs to massage the LLVM builtin? It wouldn't be appropriate to emit a call to the builtin in this case.
(In reply to comment #4) > (In reply to comment #3) > > What's the proper handling for defining a built in (not sure what the right > > term is - intrinsic?) function like abs, memcpy, etc? > > > > I imagine we should be either in one of two modes: > > > > 1) assume these things already exist and reject attempts to define them > > What happens if the call is coming from inside the house: *you* are the > implementation and are trying to provide a definition. Sorry, that's what I was trying to say - I would've thought/hoped/desired that the compiler has an explicit awareness of these two states (either you're the implementation or you're using the implementation - if the compiler is compiling the implementation then it shouldn't assume it knows anything about the "abs" function - if it is compiling a usage, it should error on an attempt to define the "abs" function) But if we don't already have that explicit awareness - yeah, we just have to muddle through. If we're muddling through, it still seems weird to rely on always_inline to fix this behavior (but I know very little of this) - shouldn't we be aware that this is a definition of 'abs' so it's got special dispensation to be distinct from __buitin_abs? Not sure - maybe should just talk to you over a drink or something > > 2) assume they don't exist and that this TU is defining them - in which case > > we shouldn't be trying to treat them as the builtin form we would provide in > > (1) > > > > no? > > What if your definition needs to massage the LLVM builtin? It wouldn't be > appropriate to emit a call to the builtin in this case. Not sure what you mean by "massage the LLVM builtin" in this context.
(In reply to comment #5) > (In reply to comment #4) > > (In reply to comment #3) > > > What's the proper handling for defining a built in (not sure what the right > > > term is - intrinsic?) function like abs, memcpy, etc? > > > > > > I imagine we should be either in one of two modes: > > > > > > 1) assume these things already exist and reject attempts to define them > > > > What happens if the call is coming from inside the house: *you* are the > > implementation and are trying to provide a definition. > > Sorry, that's what I was trying to say - I would've thought/hoped/desired > that the compiler has an explicit awareness of these two states (either > you're the implementation or you're using the implementation - if the > compiler is compiling the implementation then it shouldn't assume it knows > anything about the "abs" function - if it is compiling a usage, it should > error on an attempt to define the "abs" function) > > But if we don't already have that explicit awareness - yeah, we just have to > muddle through. If we're muddling through, it still seems weird to rely on > always_inline to fix this behavior (but I know very little of this) - > shouldn't we be aware that this is a definition of 'abs' so it's got special > dispensation to be distinct from __buitin_abs? Not sure - maybe should just > talk to you over a drink or something > > > > 2) assume they don't exist and that this TU is defining them - in which case > > > we shouldn't be trying to treat them as the builtin form we would provide in > > > (1) > > > > > > no? > > > > What if your definition needs to massage the LLVM builtin? It wouldn't be > > appropriate to emit a call to the builtin in this case. > > Not sure what you mean by "massage the LLVM builtin" in this context. Imagine a builtin called 'foo' and it's builtin, '__builtin_foo'. Suppose we'd like to define 'foo' as such: return foo(int a, int b) { __builtin_foo(b, a); } In this case, the builtin does not precisely match the arguments because it permutes them. We cannot simply call the builtin.
(In reply to comment #6) > (In reply to comment #5) > > (In reply to comment #4) > > > (In reply to comment #3) > > > > What's the proper handling for defining a built in (not sure what the right > > > > term is - intrinsic?) function like abs, memcpy, etc? > > > > > > > > I imagine we should be either in one of two modes: > > > > > > > > 1) assume these things already exist and reject attempts to define them > > > > > > What happens if the call is coming from inside the house: *you* are the > > > implementation and are trying to provide a definition. > > > > Sorry, that's what I was trying to say - I would've thought/hoped/desired > > that the compiler has an explicit awareness of these two states (either > > you're the implementation or you're using the implementation - if the > > compiler is compiling the implementation then it shouldn't assume it knows > > anything about the "abs" function - if it is compiling a usage, it should > > error on an attempt to define the "abs" function) > > > > But if we don't already have that explicit awareness - yeah, we just have to > > muddle through. If we're muddling through, it still seems weird to rely on > > always_inline to fix this behavior (but I know very little of this) - > > shouldn't we be aware that this is a definition of 'abs' so it's got special > > dispensation to be distinct from __buitin_abs? Not sure - maybe should just > > talk to you over a drink or something > > > > > > 2) assume they don't exist and that this TU is defining them - in which case > > > > we shouldn't be trying to treat them as the builtin form we would provide in > > > > (1) > > > > > > > > no? > > > > > > What if your definition needs to massage the LLVM builtin? It wouldn't be > > > appropriate to emit a call to the builtin in this case. > > > > Not sure what you mean by "massage the LLVM builtin" in this context. > > Imagine a builtin called 'foo' and it's builtin, '__builtin_foo'. Suppose > we'd like to define 'foo' as such: > > return foo(int a, int b) { > __builtin_foo(b, a); > } > > In this case, the builtin does not precisely match the arguments because it > permutes them. We cannot simply call the builtin. Sort of follow but don't. Depending on who "we" are in those sentences. The compiler already has a hardcoded assumption that foo(x, y) can be replaced by __builtin_foo(x, y) right? So if we're in a translation unit that defines foo, I assume we just accept that however foo is defined is OK by us. Up to the library implementation to provide equivalent semantics using our builtins or otherwise... eh, I'm missing steps - don't really understand whether a library implementation would use the compiler builtins to define their canonical definition. In any case - if the compiler is compiling the library implementation, I would imagine that the compiler would, for that translation unit, assume it knows nothing about the builtin - just treat it like any other function. (I would sort of hope it was built in a particular mode where it didn't have to decide on this assumption lazily - I thought that was what -fstandalone or the like were for)
(In reply to comment #7) > (In reply to comment #6) > > (In reply to comment #5) > > > (In reply to comment #4) > > > > (In reply to comment #3) > > > > > What's the proper handling for defining a built in (not sure what the right > > > > > term is - intrinsic?) function like abs, memcpy, etc? > > > > > > > > > > I imagine we should be either in one of two modes: > > > > > > > > > > 1) assume these things already exist and reject attempts to define them > > > > > > > > What happens if the call is coming from inside the house: *you* are the > > > > implementation and are trying to provide a definition. > > > > > > Sorry, that's what I was trying to say - I would've thought/hoped/desired > > > that the compiler has an explicit awareness of these two states (either > > > you're the implementation or you're using the implementation - if the > > > compiler is compiling the implementation then it shouldn't assume it knows > > > anything about the "abs" function - if it is compiling a usage, it should > > > error on an attempt to define the "abs" function) > > > > > > But if we don't already have that explicit awareness - yeah, we just have to > > > muddle through. If we're muddling through, it still seems weird to rely on > > > always_inline to fix this behavior (but I know very little of this) - > > > shouldn't we be aware that this is a definition of 'abs' so it's got special > > > dispensation to be distinct from __buitin_abs? Not sure - maybe should just > > > talk to you over a drink or something > > > > > > > > 2) assume they don't exist and that this TU is defining them - in which case > > > > > we shouldn't be trying to treat them as the builtin form we would provide in > > > > > (1) > > > > > > > > > > no? > > > > > > > > What if your definition needs to massage the LLVM builtin? It wouldn't be > > > > appropriate to emit a call to the builtin in this case. > > > > > > Not sure what you mean by "massage the LLVM builtin" in this context. > > > > Imagine a builtin called 'foo' and it's builtin, '__builtin_foo'. Suppose > > we'd like to define 'foo' as such: > > > > return foo(int a, int b) { > > __builtin_foo(b, a); > > } > > > > In this case, the builtin does not precisely match the arguments because it > > permutes them. We cannot simply call the builtin. > > Sort of follow but don't. Depending on who "we" are in those sentences. > > The compiler already has a hardcoded assumption that foo(x, y) can be > replaced by __builtin_foo(x, y) right? So if we're in a translation unit > that defines foo, I assume we just accept that however foo is defined is OK > by us. Up to the library implementation to provide equivalent semantics > using our builtins or otherwise... > > eh, I'm missing steps - don't really understand whether a library > implementation would use the compiler builtins to define their canonical > definition. > > In any case - if the compiler is compiling the library implementation, I > would imagine that the compiler would, for that translation unit, assume it > knows nothing about the builtin - just treat it like any other function. (I > would sort of hope it was built in a particular mode where it didn't have to > decide on this assumption lazily - I thought that was what -fstandalone or > the like were for) Just because we decided to name a builtin __builtin_prefetch does not give us the right to dictate the prototype (and internal mechanics) of a function called prefetch as prefetch is not specified by any standard.
> Just because we decided to name a builtin __builtin_prefetch does not give > us the right to dictate the prototype (and internal mechanics) of a function > called prefetch as prefetch is not specified by any standard. OK, maybe this is where I got off the rails. If we have no ownership over the function called "prefetch" then why would we consider it to be trivially recursive? Why would the fix to this have anything to do with always_inlineness of such a function? Shouldn't we just be more agnostic to functions named "prefetch"? (or any other suffix of one of our builtins)
Fixed in r240735.