[AVR] Use the correct address space for non-prototyped function calls

[aykevl]

Summary

Some function declarations like this:

void foo();

do not have a type declaration, for that you'd use:

void foo(void);

Clang internally bitcasts the variadic function declaration to a function pointer, but doesn't use the correct address space on AVR. This commit fixes that.

This fix is necessary to let Clang compile compiler-rt for AVR.

---

Two remarks on this diff:

1. I wasn't sure whether there is a way to set the address space on a function. I at least couldn't seem to get it to work. If it is possible to set the address space, I think the fix is slightly wrong and should somehow get the address space from the function declaration instead of using getProgramAddressSpace (please let me know how or where to find this information).
2. I hoped there was a test file already where I could include this test, but at least CodeGen/address-space.c seemed a bit too complex to also test with AVR (for example, int is 16-bit on AVR).

[rjmccall]

clang/lib/CodeGen/CGExpr.cpp

5051

CalleePtr->getType()->getPointerAddressSpace() should be fine.

[aykevl]

Update to use the function pointer address space from the function declaration.

(This is slightly different from what @rjmccall suggested but I think this is more readable).

[rjmccall]

WFM.

[MaskRay]

This fix is necessary to let Clang compile compiler-rt for AVR.

This suggests that some compiler-rt declarations do not follow the best practice... What are they?

clang/test/CodeGen/address-space-avr.c

4

Add a comment what happens here?

[aykevl]

In D78125#1982471, @MaskRay wrote:

This fix is necessary to let Clang compile compiler-rt for AVR.

This suggests that some compiler-rt declarations do not follow the best practice... What are they?

The calls come from __addXf3__ (in fp_add_impl.inc). This is what I get when I dump the bitcast:

i16 () addrspace(1)* bitcast (i16 (...) addrspace(1)* @__fe_getround to i16 () addrspace(1)*)
i16 () addrspace(1)* bitcast (i16 (...) addrspace(1)* @__fe_raise_inexact to i16 () addrspace(1)*)
i16 () addrspace(1)* bitcast (i16 (...) addrspace(1)* @__fe_getround to i16 () addrspace(1)*)
i16 () addrspace(1)* bitcast (i16 (...) addrspace(1)* @__fe_raise_inexact to i16 () addrspace(1)*)

So they must have been introduced with D57143.

[rjmccall]

clang/test/CodeGen/address-space-avr.c

4

Yeah, it'd be nice to add a comment like "Test that we call unprototyped functions from nonzero address spaces correctly." When we drop element types from pointer types, this code will need to change: FunctionPointer will start carrying an llvm::FunctionType* directly, and this code will just change that type instead of creating a bitcast. But it will still be important to test that we handle function pointers in non-zero address spaces correctly.

[aykevl]

• added comment explaining the test case

[rjmccall]

LGTM.

clang/lib/CodeGen/CGExpr.cpp

   // through an unprototyped function type works like a *non-variadic*
   // call.  The way we make this work is to cast to the exact type
   // of the promoted arguments.
   //
   // Chain calls use this same code path to add the invisible chain parameter
   // to the function type.
   if (isa<FunctionNoProtoType>(FnType) || Chain) {
     llvm::Type *CalleeTy = getTypes().GetFunctionType(FnInfo);
-    CalleeTy = CalleeTy->getPointerTo();
+    int AS = getTypes().getDataLayout().getProgramAddressSpace();
+    CalleeTy = CalleeTy->getPointerTo(AS);
 
     llvm::Value *CalleePtr = Callee.getFunctionPointer();
     CalleePtr = Builder.CreateBitCast(CalleePtr, CalleeTy, "callee.knr.cast");
     Callee.setFunctionPointer(CalleePtr);
   }
 
   llvm::CallBase *CallOrInvoke = nullptr;
   RValue Call = EmitCall(FnInfo, Callee, ReturnValue, Args, &CallOrInvoke,

clang/test/CodeGen/address-space-avr.c

+// RUN: %clang_cc1 -triple avr -emit-llvm < %s | FileCheck %s
+
+// CHECK: define void @bar() addrspace(1)
+// CHECK: call addrspace(1) void bitcast (void (...) addrspace(1)* @foo to void (i16) addrspace(1)*)(i16 3)
+// CHECK: declare void @foo(...) addrspace(1)
+void foo();
+void bar(void) {
+  foo(3);
+}