[PATCH] D84602: [MSP430] Expose msp430_builtin calling convention to C code

[Anatoly Trosinenko via Phabricator via cfe-commits]

atrosinenko added inline comments.


================
Comment at: clang/include/clang/Basic/Attr.td:1465
+  let Spellings = [Clang<"msp430_builtin">];
+  let Documentation = [Undocumented];
+}
----------------
aaron.ballman wrote:
> No new, undocumented attributes, please. Or is this attribute not expected to be used by users? (If it's not to be used by end users, is there a way we can skip exposing the attribute in the frontend and automatically generate the LLVM calling convention when lowering the builtin?)
Initially, it was not exposed and was just emitted according to the [MSP430 EABI document](https://www.ti.com/lit/an/slaa534a/slaa534a.pdf), Section 6.3, as calls to `libgcc`. Now, when porting the `builtins` library of `compiler-rt`, I had to be able to
* define **some of** functions as a library function with a special calling convention
* declare external functions to be explicitly called from corresponding unit tests

So, it is not expected to mimic some existing attribute or be used by end users. Frankly speaking, it is possible it can change the meaning when finally wiring together all parts of MSP430 compiler-rt port. Ultimately, this is expected to be wired into {D84636}.

At the first glance, msp430-gcc just knows these functions by names and sets CC accordingly. This could technically solve my problem as well but looks quite hackish to me.


================
Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:1233
     return llvm::dwarf::DW_CC_LLVM_X86RegCall;
+  // TODO case CC_MSP430Builtin:
   }
----------------
aaron.ballman wrote:
> This is likely to cause -Werror failures because the switch won't be fully covered. Are you planning to do this TODO as part of this patch, or in a follow up?
> Are you planning to do this TODO as part of this patch, or in a follow up?

It depends on how large that change would be.

I first need to find out how such calling convention identifiers are issued (or maybe there already exist one from GCC). I see they are declared inside the `llvm/include/llvm/BinaryFormat/Dwarf.def`.



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D84602/new/

https://reviews.llvm.org/D84602