[PATCH] D149867: [Clang][M68k] Add Clang support for the new M68k_RTD CC

[Min-Yih Hsu via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGfd4f96290ac9: [Clang][M68k] Add Clang support for the new 
M68k_RTD CC (authored by myhsu).

Changed prior to commit:
  https://reviews.llvm.org/D149867?vs=552596&id=557711#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149867

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang-c/Index.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Basic/Specifiers.h
  clang/include/clang/Driver/Options.td
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/Type.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/Basic/Targets/M68k.cpp
  clang/lib/Basic/Targets/M68k.h
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/CodeGen/mrtd.c
  clang/test/CodeGenCXX/default_calling_conv.cpp
  clang/test/CodeGenCXX/m68k-rtdcall.cpp
  clang/test/Sema/m68k-rtdcall.c
  clang/test/SemaCXX/m68k-rtdcall.cpp
  clang/tools/libclang/CXType.cpp

Index: clang/tools/libclang/CXType.cpp
===
--- clang/tools/libclang/CXType.cpp
+++ clang/tools/libclang/CXType.cpp
@@ -678,6 +678,7 @@
   TCALLINGCONV(SwiftAsync);
   TCALLINGCONV(PreserveMost);
   TCALLINGCONV(PreserveAll);
+  TCALLINGCONV(M68kRTD);
 case CC_SpirFunction: return CXCallingConv_Unexposed;
 case CC_AMDGPUKernelCall: return CXCallingConv_Unexposed;
 case CC_OpenCLKernel: return CXCallingConv_Unexposed;
Index: clang/test/SemaCXX/m68k-rtdcall.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/m68k-rtdcall.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -triple m68k-linux-gnu -fsyntax-only %s
+
+class A {
+public:
+  void __attribute__((m68k_rtd)) member() {}
+};
+
+void test() {
+  A a;
+  a.member();
+
+  auto f = [](int b) __attribute__((m68k_rtd)) {};
+  f(87);
+};
Index: clang/test/Sema/m68k-rtdcall.c
===
--- /dev/null
+++ clang/test/Sema/m68k-rtdcall.c
@@ -0,0 +1,46 @@
+// RUN: %clang_cc1 -triple m68k-unknown-unknown -mrtd -std=c89 -verify -verify=rtd %s
+// RUN: %clang_cc1 -triple m68k-unknown-unknown -std=c89 -verify -verify=nortd %s
+
+// rtd-error@+2 {{function with no prototype cannot use the m68k_rtd calling convention}}
+void foo(int arg) {
+  bar(arg);
+}
+
+// nortd-note@+4 {{previous declaration is here}}
+// nortd-error@+4 {{function declared 'm68k_rtd' here was previously declared without calling convention}}
+// nortd-note@+4 {{previous declaration is here}}
+// nortd-error@+4 {{function declared 'm68k_rtd' here was previously declared without calling convention}}
+void nonvariadic1(int a, int b, int c);
+void __attribute__((m68k_rtd)) nonvariadic1(int a, int b, int c);
+void nonvariadic2(int a, int b, int c);
+void __attribute__((m68k_rtd)) nonvariadic2(int a, int b, int c) { }
+
+// expected-error@+2 {{variadic function cannot use m68k_rtd calling convention}}
+void variadic(int a, ...);
+void __attribute__((m68k_rtd)) variadic(int a, ...);
+
+// rtd-note@+2 {{previous declaration is here}}
+// rtd-error@+2 {{redeclaration of 'a' with a different type: 'void ((*))(int, int) __attribute__((cdecl))' vs 'void (*)(int, int) __attribute__((m68k_rtd))'}}
+extern void (*a)(int, int);
+__attribute__((cdecl)) extern void (*a)(int, int);
+
+extern void (*b)(int, ...);
+__attribute__((cdecl)) extern void (*b)(int, ...);
+
+// nortd-note@+2 {{previous declaration is here}}
+// nortd-error@+2 {{redeclaration of 'c' with a different type: 'void ((*))(int, int) __attribute__((m68k_rtd))' vs 'void (*)(int, int)'}}
+extern void (*c)(int, int);
+__attribute__((m68k_rtd)) extern void (*c)(int, int);
+
+// expected-error@+2 {{variadic function cannot use m68k_rtd calling convention}}
+extern void (*d)(int, ...);
+__attribute__((m68k_rtd)) extern void (*d)(int, ...);
+
+// expected-warning@+1 {{'m68k_rtd' only applies to function types; type here is 'int'}}
+__attribute__((m68k_rtd)) static int g = 0;
+
+// expected-error@+1 {{'m68k_rtd' attribute takes no arguments}}
+void __attribute__((m68k_rtd("invalid"))) z(int a);
+
+// expected-error@+1 {{function with no prototype cannot use the m68k_rtd calling convention}}
+void __attribute__((m68k_rtd)) e();
Index: clang/test/CodeGenCXX/m68k-rtdcall.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/m68k-rtdcall.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -triple m68k-linu

[PATCH] D149867: [Clang][M68k] Add Clang support for the new M68k_RTD CC

[Jessica Clarke via Phabricator via cfe-commits]

jrtc27 added a comment.

Posting on GitHub loses all context; please don't do that. Whose review are you 
looking for? You've addressed my feedback and you had approval from Aaron on 
the prior version, with the latest version just being a few minor tweaks to 
address the couple of comments I had left on the approved version.


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

https://reviews.llvm.org/D149867

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D149867: [Clang][M68k] Add Clang support for the new M68k_RTD CC

[John Paul Adrian Glaubitz via Phabricator via cfe-commits]

glaubitz added a comment.

Maybe these patches need to be reposted to Github as they seem to be ignored 
here now after the swtich from Phabricator.


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

https://reviews.llvm.org/D149867

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D149867: [Clang][M68k] Add Clang support for the new M68k_RTD CC

[Sheng via Phabricator via cfe-commits]

0x59616e added a comment.

ping.


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

https://reviews.llvm.org/D149867

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D149867: [Clang][M68k] Add Clang support for the new M68k_RTD CC

[John Paul Adrian Glaubitz via Phabricator via cfe-commits]

glaubitz added a comment.

Any news on this? I guess it would be nice to get the remaining m68k patches 
merged before the Phrabricator shutdown.


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

https://reviews.llvm.org/D149867

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D149867: [Clang][M68k] Add Clang support for the new M68k_RTD CC

[John Paul Adrian Glaubitz via Phabricator via cfe-commits]

glaubitz added a comment.

In D149867#4612589 , @myhsu wrote:

> In D149867#4603853 , @aaron.ballman 
> wrote:
>
>> In D149867#4544489 , @myhsu wrote:
>>
>>> Sorry I was busy on my phd defense (which I passed!) in the past month. 
>>> I'll get back to this next week.
>>
>> Congratulations!! 🎉
>
> Thank you!

Oh man, I completely missed that.

Congratulations, Min! That must be a huge relieve!


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

https://reviews.llvm.org/D149867

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D149867: [Clang][M68k] Add Clang support for the new M68k_RTD CC

[Min-Yih Hsu via Phabricator via cfe-commits]

myhsu added a comment.

In D149867#4603853 , @aaron.ballman 
wrote:

> In D149867#4544489 , @myhsu wrote:
>
>> Sorry I was busy on my phd defense (which I passed!) in the past month. I'll 
>> get back to this next week.
>
> Congratulations!! 🎉

Thank you!


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

https://reviews.llvm.org/D149867

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D149867: [Clang][M68k] Add Clang support for the new M68k_RTD CC

[Min-Yih Hsu via Phabricator via cfe-commits]

myhsu added inline comments.



Comment at: clang/test/CodeGen/mrtd.c:10
+// X86: call x86_stdcallcc i32 @bar(
+#ifndef mc68000
   bar(arg);

jrtc27 wrote:
> Uh, this shouldn't be defined in ISO C; GCC's using builtin_define_std, so 
> you should only get `__mc68000` and `__mc68000__` for ISO C, i.e. using 
> `DefineStd("mc68000")` in libClangBasic and let it add the underscored 
> variants needed
Thanks for pointing out. I'll put the changes to using `DefineStd` in a 
separate patch.


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

https://reviews.llvm.org/D149867

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D149867: [Clang][M68k] Add Clang support for the new M68k_RTD CC

[Min-Yih Hsu via Phabricator via cfe-commits]

myhsu updated this revision to Diff 552596.
myhsu marked 2 inline comments as done.
myhsu added a comment.

Addressed comments in `test/CodeGen/mrtd.c`


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

https://reviews.llvm.org/D149867

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang-c/Index.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Basic/Specifiers.h
  clang/include/clang/Driver/Options.td
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/Type.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/Basic/Targets/M68k.cpp
  clang/lib/Basic/Targets/M68k.h
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/CodeGen/mrtd.c
  clang/test/CodeGenCXX/default_calling_conv.cpp
  clang/test/CodeGenCXX/m68k-rtdcall.cpp
  clang/test/Sema/m68k-rtdcall.c
  clang/test/SemaCXX/m68k-rtdcall.cpp
  clang/tools/libclang/CXType.cpp

Index: clang/tools/libclang/CXType.cpp
===
--- clang/tools/libclang/CXType.cpp
+++ clang/tools/libclang/CXType.cpp
@@ -678,6 +678,7 @@
   TCALLINGCONV(SwiftAsync);
   TCALLINGCONV(PreserveMost);
   TCALLINGCONV(PreserveAll);
+  TCALLINGCONV(M68kRTD);
 case CC_SpirFunction: return CXCallingConv_Unexposed;
 case CC_AMDGPUKernelCall: return CXCallingConv_Unexposed;
 case CC_OpenCLKernel: return CXCallingConv_Unexposed;
Index: clang/test/SemaCXX/m68k-rtdcall.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/m68k-rtdcall.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -triple m68k-linux-gnu -fsyntax-only %s
+
+class A {
+public:
+  void __attribute__((m68k_rtd)) member() {}
+};
+
+void test() {
+  A a;
+  a.member();
+
+  auto f = [](int b) __attribute__((m68k_rtd)) {};
+  f(87);
+};
Index: clang/test/Sema/m68k-rtdcall.c
===
--- /dev/null
+++ clang/test/Sema/m68k-rtdcall.c
@@ -0,0 +1,46 @@
+// RUN: %clang_cc1 -triple m68k-unknown-unknown -mrtd -std=c89 -verify -verify=rtd %s
+// RUN: %clang_cc1 -triple m68k-unknown-unknown -std=c89 -verify -verify=nortd %s
+
+// rtd-error@+2 {{function with no prototype cannot use the m68k_rtd calling convention}}
+void foo(int arg) {
+  bar(arg);
+}
+
+// nortd-note@+4 {{previous declaration is here}}
+// nortd-error@+4 {{function declared 'm68k_rtd' here was previously declared without calling convention}}
+// nortd-note@+4 {{previous declaration is here}}
+// nortd-error@+4 {{function declared 'm68k_rtd' here was previously declared without calling convention}}
+void nonvariadic1(int a, int b, int c);
+void __attribute__((m68k_rtd)) nonvariadic1(int a, int b, int c);
+void nonvariadic2(int a, int b, int c);
+void __attribute__((m68k_rtd)) nonvariadic2(int a, int b, int c) { }
+
+// expected-error@+2 {{variadic function cannot use m68k_rtd calling convention}}
+void variadic(int a, ...);
+void __attribute__((m68k_rtd)) variadic(int a, ...);
+
+// rtd-note@+2 {{previous declaration is here}}
+// rtd-error@+2 {{redeclaration of 'a' with a different type: 'void ((*))(int, int) __attribute__((cdecl))' vs 'void (*)(int, int) __attribute__((m68k_rtd))'}}
+extern void (*a)(int, int);
+__attribute__((cdecl)) extern void (*a)(int, int);
+
+extern void (*b)(int, ...);
+__attribute__((cdecl)) extern void (*b)(int, ...);
+
+// nortd-note@+2 {{previous declaration is here}}
+// nortd-error@+2 {{redeclaration of 'c' with a different type: 'void ((*))(int, int) __attribute__((m68k_rtd))' vs 'void (*)(int, int)'}}
+extern void (*c)(int, int);
+__attribute__((m68k_rtd)) extern void (*c)(int, int);
+
+// expected-error@+2 {{variadic function cannot use m68k_rtd calling convention}}
+extern void (*d)(int, ...);
+__attribute__((m68k_rtd)) extern void (*d)(int, ...);
+
+// expected-warning@+1 {{'m68k_rtd' only applies to function types; type here is 'int'}}
+__attribute__((m68k_rtd)) static int g = 0;
+
+// expected-error@+1 {{'m68k_rtd' attribute takes no arguments}}
+void __attribute__((m68k_rtd("invalid"))) z(int a);
+
+// expected-error@+1 {{function with no prototype cannot use the m68k_rtd calling convention}}
+void __attribute__((m68k_rtd)) e();
Index: clang/test/CodeGenCXX/m68k-rtdcall.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/m68k-rtdcall.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -triple m68k-linux-gnu -emit-llvm -o - %s | FileCheck %s
+
+class A {
+public:
+// CHECK: define{{.*}} m68k_rtdcc void @_ZN1A6memberEv
+  void __attribute__((m68k_rtd)) member() {}
+};
+
+void test() {
+  A a;
+  a.member();
+
+// CHECK: define{{.*}} m68k_rtdcc void @"_ZZ4testvENK3$_0clEi"
+  auto f = [](int b)

[PATCH] D149867: [Clang][M68k] Add Clang support for the new M68k_RTD CC

[Jessica Clarke via Phabricator via cfe-commits]

jrtc27 added inline comments.



Comment at: clang/test/CodeGen/mrtd.c:2
+// RUN: %clang_cc1 -mrtd -triple i386-unknown-unknown -std=c89 -emit-llvm -o - 
%s 2>&1 | FileCheck --check-prefixes=CHECK,X86 %s
+// RUN: %clang_cc1 -mrtd -triple m68k-unknown-unknown -std=c89 -emit-llvm -o - 
%s 2>&1 | FileCheck --check-prefixes=CHECK,M68k %s
 

Capitalise CHECK prefixes



Comment at: clang/test/CodeGen/mrtd.c:10
+// X86: call x86_stdcallcc i32 @bar(
+#ifndef mc68000
   bar(arg);

Uh, this shouldn't be defined in ISO C; GCC's using builtin_define_std, so you 
should only get `__mc68000` and `__mc68000__` for ISO C, i.e. using 
`DefineStd("mc68000")` in libClangBasic and let it add the underscored variants 
needed


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

https://reviews.llvm.org/D149867

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D149867: [Clang][M68k] Add Clang support for the new M68k_RTD CC

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.

In D149867#4544489 , @myhsu wrote:

> Sorry I was busy on my phd defense (which I passed!) in the past month. I'll 
> get back to this next week.

Congratulations!! 🎉

LGTM!




Comment at: clang/test/Sema/m68k-mrtd.c:4-9
+#ifdef MRTD
+// expected-error@+3 {{function with no prototype cannot use the m68k_rtd 
calling convention}}
+#endif
+void foo(int arg) {
+  bar(arg);
+}

myhsu wrote:
> aaron.ballman wrote:
> > myhsu wrote:
> > > aaron.ballman wrote:
> > > > A better way to do this is to use `-verify=mrtd` on the line enabling 
> > > > rtd, and using `// rtd-error {{whatever}}` on the line being diagnosed. 
> > > > (Same comment applies throughout the file.)
> > > > 
> > > > Huh, I was unaware that implicit function declarations are using 
> > > > something other than the default calling convention (which is C, not 
> > > > m68k_rtd). Is this intentional?
> > > > Huh, I was unaware that implicit function declarations are using 
> > > > something other than the default calling convention (which is C, not 
> > > > m68k_rtd). Is this intentional?
> > > 
> > > I'm not sure if I understand you correctly, but this diagnostic is 
> > > emitted if the CC does not support variadic function call. 
> > `bar` is not declared, in C89 this causes an implicit function declaration 
> > to be generated with the signature: `extern int ident();` What surprised me 
> > is that we seem to be generating something more like this instead: 
> > `__attribute__((m68k_rtd)) int ident();` despite that not being valid.
> I understand what you meant, but the standard only says that using implicit 
> declared identifier is as if `extern int ident();` appears in the source 
> code. My interpretation is that in this case since the very source code is 
> compiled with `-mrtd`, every functions use m68k_rtd rather than C calling 
> convention. 
> 
> Another example is stdcall: i386 Clang emits a similar warning ("function 
> with no prototype cannot use the stdcall calling convention") when `-mrtd` is 
> used (to set default CC to stdcall) on the same snippet. Should `bar` was not 
> called with stdcall under the influence of `-mrtd`, the aforementioned 
> message would not be emitted in the first place.
> i386 GCC also calls `bar` with stdcall when `-mrtd` is given (though no 
> warning message is emitted).
Ahhh okay, this makes more sense now -- this matches other ways in which we set 
the default calling convention, it's just a bit more hidden from view. Here's a 
more obvious example: https://godbolt.org/z/sboxf83s6



Comment at: clang/test/Sema/m68k-mrtd.c:45
+extern void (*d)(int, ...);
+__attribute__((m68k_rtd)) extern void (*d)(int, ...);

myhsu wrote:
> aaron.ballman wrote:
> > myhsu wrote:
> > > aaron.ballman wrote:
> > > > Missing tests for:
> > > > 
> > > > * Function without a prototype
> > > > * Applying the attribute to a non-function
> > > > * Providing arguments to the attribute
> > > > * What should happen for C++ and things like member functions?
> > > > Function without a prototype
> > > 
> > > I thought the first check was testing function without a prototype.
> > > 
> > > > What should happen for C++ and things like member functions?
> > > 
> > > I believe we don't have any special handling for C++.
> > > 
> > > I addressed rest of the bullet items you mentioned, please take a look.
> > >> Function without a prototype
> > > I thought the first check was testing function without a prototype.
> > 
> > Nope, I meant something more like this:
> > ```
> > __attribute__((m68k_rtd)) void foo(); // Should error
> > __attribute__((m68k_rtd)) void bar(a, b) int a, b; {} // Should error
> > ```
> > 
> > >> What should happen for C++ and things like member functions?
> > > I believe we don't have any special handling for C++.
> > 
> > Test coverage should still exist to ensure the behavior is what you'd 
> > expect when adding the calling convention to a member function and a 
> > lambda, for example. (Both Sema and CodeGen tests for this)
> > 
> > Also missing coverage for the changes to `-fdefault-calling-conv=`
> > 
> > I'm also still wondering whether there's a way to use m68k with a Windows 
> > ABI triple (basically, do we need changes in MicrosoftMangle.cpp?)
> 
> > ```
> > __attribute__((m68k_rtd)) void bar(a, b) int a, b; {} // Should error
> > ```
> 
> Right now Clang only gives a warning about potential behavior change after 
> C23, rather than an error for this kind of syntax. Because it seems like 
> Clang doesn't check this situation against unsupported calling convention. 
> I'm not sure if we should throw the same error as `__attribute__((m68k_rtd)) 
> void foo()`.
> 
> > I'm also still wondering whether there's a way to use m68k with a Windows 
> > ABI triple (basically, do we need changes in MicrosoftMangle.cpp?)
> 
> I don't think it's w

[PATCH] D149867: [Clang][M68k] Add Clang support for the new M68k_RTD CC

[Min-Yih Hsu via Phabricator via cfe-commits]

myhsu added inline comments.



Comment at: clang/test/Sema/m68k-mrtd.c:4-9
+#ifdef MRTD
+// expected-error@+3 {{function with no prototype cannot use the m68k_rtd 
calling convention}}
+#endif
+void foo(int arg) {
+  bar(arg);
+}

aaron.ballman wrote:
> myhsu wrote:
> > aaron.ballman wrote:
> > > A better way to do this is to use `-verify=mrtd` on the line enabling 
> > > rtd, and using `// rtd-error {{whatever}}` on the line being diagnosed. 
> > > (Same comment applies throughout the file.)
> > > 
> > > Huh, I was unaware that implicit function declarations are using 
> > > something other than the default calling convention (which is C, not 
> > > m68k_rtd). Is this intentional?
> > > Huh, I was unaware that implicit function declarations are using 
> > > something other than the default calling convention (which is C, not 
> > > m68k_rtd). Is this intentional?
> > 
> > I'm not sure if I understand you correctly, but this diagnostic is emitted 
> > if the CC does not support variadic function call. 
> `bar` is not declared, in C89 this causes an implicit function declaration to 
> be generated with the signature: `extern int ident();` What surprised me is 
> that we seem to be generating something more like this instead: 
> `__attribute__((m68k_rtd)) int ident();` despite that not being valid.
I understand what you meant, but the standard only says that using implicit 
declared identifier is as if `extern int ident();` appears in the source code. 
My interpretation is that in this case since the very source code is compiled 
with `-mrtd`, every functions use m68k_rtd rather than C calling convention. 

Another example is stdcall: i386 Clang emits a similar warning ("function with 
no prototype cannot use the stdcall calling convention") when `-mrtd` is used 
(to set default CC to stdcall) on the same snippet. Should `bar` was not called 
with stdcall under the influence of `-mrtd`, the aforementioned message would 
not be emitted in the first place.
i386 GCC also calls `bar` with stdcall when `-mrtd` is given (though no warning 
message is emitted).



Comment at: clang/test/Sema/m68k-mrtd.c:45
+extern void (*d)(int, ...);
+__attribute__((m68k_rtd)) extern void (*d)(int, ...);

aaron.ballman wrote:
> myhsu wrote:
> > aaron.ballman wrote:
> > > Missing tests for:
> > > 
> > > * Function without a prototype
> > > * Applying the attribute to a non-function
> > > * Providing arguments to the attribute
> > > * What should happen for C++ and things like member functions?
> > > Function without a prototype
> > 
> > I thought the first check was testing function without a prototype.
> > 
> > > What should happen for C++ and things like member functions?
> > 
> > I believe we don't have any special handling for C++.
> > 
> > I addressed rest of the bullet items you mentioned, please take a look.
> >> Function without a prototype
> > I thought the first check was testing function without a prototype.
> 
> Nope, I meant something more like this:
> ```
> __attribute__((m68k_rtd)) void foo(); // Should error
> __attribute__((m68k_rtd)) void bar(a, b) int a, b; {} // Should error
> ```
> 
> >> What should happen for C++ and things like member functions?
> > I believe we don't have any special handling for C++.
> 
> Test coverage should still exist to ensure the behavior is what you'd expect 
> when adding the calling convention to a member function and a lambda, for 
> example. (Both Sema and CodeGen tests for this)
> 
> Also missing coverage for the changes to `-fdefault-calling-conv=`
> 
> I'm also still wondering whether there's a way to use m68k with a Windows ABI 
> triple (basically, do we need changes in MicrosoftMangle.cpp?)

> ```
> __attribute__((m68k_rtd)) void bar(a, b) int a, b; {} // Should error
> ```

Right now Clang only gives a warning about potential behavior change after C23, 
rather than an error for this kind of syntax. Because it seems like Clang 
doesn't check this situation against unsupported calling convention. I'm not 
sure if we should throw the same error as `__attribute__((m68k_rtd)) void 
foo()`.

> I'm also still wondering whether there's a way to use m68k with a Windows ABI 
> triple (basically, do we need changes in MicrosoftMangle.cpp?)

I don't think it's worth it to support m68k with Windows ABI as this 
combination never existed (and unlikely to happen in the future)


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

https://reviews.llvm.org/D149867

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D149867: [Clang][M68k] Add Clang support for the new M68k_RTD CC

[Min-Yih Hsu via Phabricator via cfe-commits]

myhsu updated this revision to Diff 551797.
myhsu marked 4 inline comments as done.
myhsu added a comment.

- Add more C++ tests for `m68k_rtd`, both Sema and CodeGen ones
- Addressed other comments


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

https://reviews.llvm.org/D149867

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang-c/Index.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Basic/Specifiers.h
  clang/include/clang/Driver/Options.td
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/Type.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/Basic/Targets/M68k.cpp
  clang/lib/Basic/Targets/M68k.h
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/CodeGen/mrtd.c
  clang/test/CodeGenCXX/default_calling_conv.cpp
  clang/test/CodeGenCXX/m68k-rtdcall.cpp
  clang/test/Sema/m68k-rtdcall.c
  clang/test/SemaCXX/m68k-rtdcall.cpp
  clang/tools/libclang/CXType.cpp

Index: clang/tools/libclang/CXType.cpp
===
--- clang/tools/libclang/CXType.cpp
+++ clang/tools/libclang/CXType.cpp
@@ -678,6 +678,7 @@
   TCALLINGCONV(SwiftAsync);
   TCALLINGCONV(PreserveMost);
   TCALLINGCONV(PreserveAll);
+  TCALLINGCONV(M68kRTD);
 case CC_SpirFunction: return CXCallingConv_Unexposed;
 case CC_AMDGPUKernelCall: return CXCallingConv_Unexposed;
 case CC_OpenCLKernel: return CXCallingConv_Unexposed;
Index: clang/test/SemaCXX/m68k-rtdcall.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/m68k-rtdcall.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -triple m68k-linux-gnu -fsyntax-only %s
+
+class A {
+public:
+  void __attribute__((m68k_rtd)) member() {}
+};
+
+void test() {
+  A a;
+  a.member();
+
+  auto f = [](int b) __attribute__((m68k_rtd)) {};
+  f(87);
+};
Index: clang/test/Sema/m68k-rtdcall.c
===
--- /dev/null
+++ clang/test/Sema/m68k-rtdcall.c
@@ -0,0 +1,46 @@
+// RUN: %clang_cc1 -triple m68k-unknown-unknown -mrtd -std=c89 -verify -verify=rtd %s
+// RUN: %clang_cc1 -triple m68k-unknown-unknown -std=c89 -verify -verify=nortd %s
+
+// rtd-error@+2 {{function with no prototype cannot use the m68k_rtd calling convention}}
+void foo(int arg) {
+  bar(arg);
+}
+
+// nortd-note@+4 {{previous declaration is here}}
+// nortd-error@+4 {{function declared 'm68k_rtd' here was previously declared without calling convention}}
+// nortd-note@+4 {{previous declaration is here}}
+// nortd-error@+4 {{function declared 'm68k_rtd' here was previously declared without calling convention}}
+void nonvariadic1(int a, int b, int c);
+void __attribute__((m68k_rtd)) nonvariadic1(int a, int b, int c);
+void nonvariadic2(int a, int b, int c);
+void __attribute__((m68k_rtd)) nonvariadic2(int a, int b, int c) { }
+
+// expected-error@+2 {{variadic function cannot use m68k_rtd calling convention}}
+void variadic(int a, ...);
+void __attribute__((m68k_rtd)) variadic(int a, ...);
+
+// rtd-note@+2 {{previous declaration is here}}
+// rtd-error@+2 {{redeclaration of 'a' with a different type: 'void ((*))(int, int) __attribute__((cdecl))' vs 'void (*)(int, int) __attribute__((m68k_rtd))'}}
+extern void (*a)(int, int);
+__attribute__((cdecl)) extern void (*a)(int, int);
+
+extern void (*b)(int, ...);
+__attribute__((cdecl)) extern void (*b)(int, ...);
+
+// nortd-note@+2 {{previous declaration is here}}
+// nortd-error@+2 {{redeclaration of 'c' with a different type: 'void ((*))(int, int) __attribute__((m68k_rtd))' vs 'void (*)(int, int)'}}
+extern void (*c)(int, int);
+__attribute__((m68k_rtd)) extern void (*c)(int, int);
+
+// expected-error@+2 {{variadic function cannot use m68k_rtd calling convention}}
+extern void (*d)(int, ...);
+__attribute__((m68k_rtd)) extern void (*d)(int, ...);
+
+// expected-warning@+1 {{'m68k_rtd' only applies to function types; type here is 'int'}}
+__attribute__((m68k_rtd)) static int g = 0;
+
+// expected-error@+1 {{'m68k_rtd' attribute takes no arguments}}
+void __attribute__((m68k_rtd("invalid"))) z(int a);
+
+// expected-error@+1 {{function with no prototype cannot use the m68k_rtd calling convention}}
+void __attribute__((m68k_rtd)) e();
Index: clang/test/CodeGenCXX/m68k-rtdcall.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/m68k-rtdcall.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -triple m68k-linux-gnu -emit-llvm -o - %s | FileCheck %s
+
+class A {
+public:
+// CHECK: define{{.*}} m68k_rtdcc void @_ZN1A6memberEv
+  void __attribute__((m68k_rtd)) member() {}
+};
+
+void test() {
+  A a;
+  a.member();
+
+// CHECK: define{{.*}} m68k_rtdcc voi

[PATCH] D149867: [Clang][M68k] Add Clang support for the new M68k_RTD CC

[Min-Yih Hsu via Phabricator via cfe-commits]

myhsu added a comment.

In D149867#4544281 , @glaubitz wrote:

> Any update on this?

Sorry I was busy on my phd defense (which I passed!) in the past month. I'll 
get back to this next week.


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

https://reviews.llvm.org/D149867

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D149867: [Clang][M68k] Add Clang support for the new M68k_RTD CC

[John Paul Adrian Glaubitz via Phabricator via cfe-commits]

glaubitz added a comment.

Any update on this?


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

https://reviews.llvm.org/D149867

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D149867: [Clang][M68k] Add Clang support for the new M68k_RTD CC

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Also, it looks like precommit CI found a relevant failure that should be 
investigated.


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

https://reviews.llvm.org/D149867

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D149867: [Clang][M68k] Add Clang support for the new M68k_RTD CC

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Sorry for the long delay in review on this, it slipped through the cracks for 
me.




Comment at: clang/lib/AST/TypePrinter.cpp:1861
 break;
+  case attr::M68kRTD: OS << "m68k_rtd"; break;
   case attr::NoDeref:





Comment at: clang/test/Sema/m68k-mrtd.c:4-9
+#ifdef MRTD
+// expected-error@+3 {{function with no prototype cannot use the m68k_rtd 
calling convention}}
+#endif
+void foo(int arg) {
+  bar(arg);
+}

myhsu wrote:
> aaron.ballman wrote:
> > A better way to do this is to use `-verify=mrtd` on the line enabling rtd, 
> > and using `// rtd-error {{whatever}}` on the line being diagnosed. (Same 
> > comment applies throughout the file.)
> > 
> > Huh, I was unaware that implicit function declarations are using something 
> > other than the default calling convention (which is C, not m68k_rtd). Is 
> > this intentional?
> > Huh, I was unaware that implicit function declarations are using something 
> > other than the default calling convention (which is C, not m68k_rtd). Is 
> > this intentional?
> 
> I'm not sure if I understand you correctly, but this diagnostic is emitted if 
> the CC does not support variadic function call. 
`bar` is not declared, in C89 this causes an implicit function declaration to 
be generated with the signature: `extern int ident();` What surprised me is 
that we seem to be generating something more like this instead: 
`__attribute__((m68k_rtd)) int ident();` despite that not being valid.



Comment at: clang/test/Sema/m68k-mrtd.c:45
+extern void (*d)(int, ...);
+__attribute__((m68k_rtd)) extern void (*d)(int, ...);

myhsu wrote:
> aaron.ballman wrote:
> > Missing tests for:
> > 
> > * Function without a prototype
> > * Applying the attribute to a non-function
> > * Providing arguments to the attribute
> > * What should happen for C++ and things like member functions?
> > Function without a prototype
> 
> I thought the first check was testing function without a prototype.
> 
> > What should happen for C++ and things like member functions?
> 
> I believe we don't have any special handling for C++.
> 
> I addressed rest of the bullet items you mentioned, please take a look.
>> Function without a prototype
> I thought the first check was testing function without a prototype.

Nope, I meant something more like this:
```
__attribute__((m68k_rtd)) void foo(); // Should error
__attribute__((m68k_rtd)) void bar(a, b) int a, b; {} // Should error
```

>> What should happen for C++ and things like member functions?
> I believe we don't have any special handling for C++.

Test coverage should still exist to ensure the behavior is what you'd expect 
when adding the calling convention to a member function and a lambda, for 
example. (Both Sema and CodeGen tests for this)

Also missing coverage for the changes to `-fdefault-calling-conv=`

I'm also still wondering whether there's a way to use m68k with a Windows ABI 
triple (basically, do we need changes in MicrosoftMangle.cpp?)


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

https://reviews.llvm.org/D149867

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D149867: [Clang][M68k] Add Clang support for the new M68k_RTD CC

[Min-Yih Hsu via Phabricator via cfe-commits]

myhsu added inline comments.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:559
+emitError |= DefaultCC == LangOptions::DCC_StdCall &&
+ Arch != llvm::Triple::m68k && Arch != llvm::Triple::x86;
 emitError |= (DefaultCC == LangOptions::DCC_VectorCall ||

aaron.ballman wrote:
> Maybe it's too early in the morning for me to be thinking clearly, but this 
> is wrong for m68k, isn't it? If the default calling convention is stdcall and 
> the architecture is m68k, we want to emit the error, don't we?
> 
> I don't see test coverage for the change.
The idea was to use our new RTD CC whenever `-mrtd` is given to either the 
driver or the frontend. Due to some historical reasons `-mrtd` has been taken 
by i386 to specify stdcall. Specifically, DefaultCallingConvention will be set 
to `DCC_StdCall` when `-mrtd` is present. My original approach was to reuse 
this workflow and the `DCC_StdCall`, then translate to either `CC_X86StdCall` 
or `CC_M68kRTD` later in ASTContext.

Since there are many concerns on reusing DCC_StdCall, I will create another DCC 
enum value instead.



Comment at: clang/test/Sema/m68k-mrtd.c:4-9
+#ifdef MRTD
+// expected-error@+3 {{function with no prototype cannot use the m68k_rtd 
calling convention}}
+#endif
+void foo(int arg) {
+  bar(arg);
+}

aaron.ballman wrote:
> A better way to do this is to use `-verify=mrtd` on the line enabling rtd, 
> and using `// rtd-error {{whatever}}` on the line being diagnosed. (Same 
> comment applies throughout the file.)
> 
> Huh, I was unaware that implicit function declarations are using something 
> other than the default calling convention (which is C, not m68k_rtd). Is this 
> intentional?
> Huh, I was unaware that implicit function declarations are using something 
> other than the default calling convention (which is C, not m68k_rtd). Is this 
> intentional?

I'm not sure if I understand you correctly, but this diagnostic is emitted if 
the CC does not support variadic function call. 



Comment at: clang/test/Sema/m68k-mrtd.c:45
+extern void (*d)(int, ...);
+__attribute__((m68k_rtd)) extern void (*d)(int, ...);

aaron.ballman wrote:
> Missing tests for:
> 
> * Function without a prototype
> * Applying the attribute to a non-function
> * Providing arguments to the attribute
> * What should happen for C++ and things like member functions?
> Function without a prototype

I thought the first check was testing function without a prototype.

> What should happen for C++ and things like member functions?

I believe we don't have any special handling for C++.

I addressed rest of the bullet items you mentioned, please take a look.


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

https://reviews.llvm.org/D149867

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D149867: [Clang][M68k] Add Clang support for the new M68k_RTD CC

[Min-Yih Hsu via Phabricator via cfe-commits]

myhsu updated this revision to Diff 532423.
myhsu marked 5 inline comments as done.
myhsu edited the summary of this revision.
myhsu added a comment.
Herald added a subscriber: MaskRay.

- Add a new default calling convention `-fdefault-calling-conv=rtdcall` to 
model using `m68k_rtdcc` globally
- Add documents for `rtdcall`
- Add more tests on `__attribute__((m68k_rtd))`


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

https://reviews.llvm.org/D149867

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang-c/Index.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Basic/Specifiers.h
  clang/include/clang/Driver/Options.td
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/Type.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/Basic/Targets/M68k.cpp
  clang/lib/Basic/Targets/M68k.h
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/CodeGen/mrtd.c
  clang/test/Sema/m68k-mrtd.c
  clang/tools/libclang/CXType.cpp

Index: clang/tools/libclang/CXType.cpp
===
--- clang/tools/libclang/CXType.cpp
+++ clang/tools/libclang/CXType.cpp
@@ -678,6 +678,7 @@
   TCALLINGCONV(SwiftAsync);
   TCALLINGCONV(PreserveMost);
   TCALLINGCONV(PreserveAll);
+  TCALLINGCONV(M68kRTD);
 case CC_SpirFunction: return CXCallingConv_Unexposed;
 case CC_AMDGPUKernelCall: return CXCallingConv_Unexposed;
 case CC_OpenCLKernel: return CXCallingConv_Unexposed;
Index: clang/test/Sema/m68k-mrtd.c
===
--- /dev/null
+++ clang/test/Sema/m68k-mrtd.c
@@ -0,0 +1,43 @@
+// RUN: %clang_cc1 -triple m68k-unknown-unknown -mrtd -std=c89 -verify -verify=rtd %s
+// RUN: %clang_cc1 -triple m68k-unknown-unknown -std=c89 -verify -verify=nortd %s
+
+// rtd-error@+2 {{function with no prototype cannot use the m68k_rtd calling convention}}
+void foo(int arg) {
+  bar(arg);
+}
+
+// nortd-note@+4 {{previous declaration is here}}
+// nortd-error@+4 {{function declared 'm68k_rtd' here was previously declared without calling convention}}
+// nortd-note@+4 {{previous declaration is here}}
+// nortd-error@+4 {{function declared 'm68k_rtd' here was previously declared without calling convention}}
+void nonvariadic1(int a, int b, int c);
+void __attribute__((m68k_rtd)) nonvariadic1(int a, int b, int c);
+void nonvariadic2(int a, int b, int c);
+void __attribute__((m68k_rtd)) nonvariadic2(int a, int b, int c) { }
+
+// expected-error@+2 {{variadic function cannot use m68k_rtd calling convention}}
+void variadic(int a, ...);
+void __attribute__((m68k_rtd)) variadic(int a, ...);
+
+// rtd-note@+2 {{previous declaration is here}}
+// rtd-error@+2 {{redeclaration of 'a' with a different type: 'void ((*))(int, int) __attribute__((cdecl))' vs 'void (*)(int, int) __attribute__((m68k_rtd))'}}
+extern void (*a)(int, int);
+__attribute__((cdecl)) extern void (*a)(int, int);
+
+extern void (*b)(int, ...);
+__attribute__((cdecl)) extern void (*b)(int, ...);
+
+// nortd-note@+2 {{previous declaration is here}}
+// nortd-error@+2 {{redeclaration of 'c' with a different type: 'void ((*))(int, int) __attribute__((m68k_rtd))' vs 'void (*)(int, int)'}}
+extern void (*c)(int, int);
+__attribute__((m68k_rtd)) extern void (*c)(int, int);
+
+// expected-error@+2 {{variadic function cannot use m68k_rtd calling convention}}
+extern void (*d)(int, ...);
+__attribute__((m68k_rtd)) extern void (*d)(int, ...);
+
+// expected-warning@+1 {{'m68k_rtd' only applies to function types; type here is 'int'}}
+__attribute__((m68k_rtd)) static int g = 0;
+
+// expected-error@+1 {{'m68k_rtd' attribute takes no arguments}}
+void __attribute__((m68k_rtd("invalid"))) z(int a);
Index: clang/test/CodeGen/mrtd.c
===
--- clang/test/CodeGen/mrtd.c
+++ clang/test/CodeGen/mrtd.c
@@ -1,20 +1,24 @@
-// RUN: %clang_cc1 -mrtd -triple i386-unknown-unknown -std=c89 -emit-llvm -o - %s 2>&1 | FileCheck %s
-
-// CHECK: mrtd.c:10:3: warning: function with no prototype cannot use the stdcall calling convention
+// RUN: %clang_cc1 -mrtd -triple i386-unknown-unknown -std=c89 -emit-llvm -o - %s 2>&1 | FileCheck --check-prefixes=CHECK,X86 %s
+// RUN: %clang_cc1 -mrtd -triple m68k-unknown-unknown -std=c89 -emit-llvm -o - %s 2>&1 | FileCheck --check-prefixes=CHECK,M68k %s
 
 void baz(int arg);
 
-// CHECK: define{{.*}} x86_stdcallcc void @foo(i32 noundef %arg) [[NUW:#[0-9]+]]
+// X86: define{{.*}} x86_stdcallcc void @foo(i32 noundef %arg) [[NUW:#[0-9]+]]
+// M68k: define{{.*}} m68k_rtdcc void @foo(i32 noundef %arg)
 void foo(int arg) {
-// CHECK: call x86_stdcallcc i32 @bar(
+// X86: call x86_stdcallcc i32 @ba

[PATCH] D149867: [Clang][M68k] Add Clang support for the new M68k_RTD CC

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

This should also come with a release note for the changes.




Comment at: clang/lib/AST/ASTContext.cpp:11998-11999
+  return CC_X86StdCall;
+else if (T.getArch() == llvm::Triple::m68k)
+  return CC_M68kRTD;
+  }

This looks wrong to me -- if the language option is calling for stdcall, saying 
"here's m68k_rtd instead" does not match caller expectations.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:559
+emitError |= DefaultCC == LangOptions::DCC_StdCall &&
+ Arch != llvm::Triple::m68k && Arch != llvm::Triple::x86;
 emitError |= (DefaultCC == LangOptions::DCC_VectorCall ||

Maybe it's too early in the morning for me to be thinking clearly, but this is 
wrong for m68k, isn't it? If the default calling convention is stdcall and the 
architecture is m68k, we want to emit the error, don't we?

I don't see test coverage for the change.



Comment at: clang/lib/Sema/SemaType.cpp:7960-7961
 if (FnP && FnP->isVariadic()) {
-  // stdcall and fastcall are ignored with a warning for GCC and MS
-  // compatibility.
-  if (CC == CC_X86StdCall || CC == CC_X86FastCall)
+  // stdcall, fastcall, and m68k's RTD are ignored with a warning for GCC
+  // and MS compatibility.
+  if (CC == CC_X86StdCall || CC == CC_X86FastCall || CC == CC_M68kRTD)

Missing test coverage, but also, MSVC supports m68k RTD? I would assume we'd 
error on this situation given how we document the attribute as not supporting 
this.



Comment at: clang/test/CodeGen/mrtd.c:4
 
-// CHECK: mrtd.c:10:3: warning: function with no prototype cannot use the 
stdcall calling convention
+// CHECK: mrtd.c:13:3: warning: function with no prototype cannot use the 
stdcall calling convention
 

myhsu wrote:
> jrtc27 wrote:
> > Ew... this should be using -verify and `// expected-warning {{...}}`, then 
> > the line number is relative to the comment's location. Or, really, it 
> > should be in the Sema test...
> > 
> > Plus is it correct to call this stdcall on m68k? The GCC manpage only 
> > mentions it in the x86 option description, not the m68k one.
> Now this check is moved to `test/Sema/m68k-mrtd.c`
This check should be removed here -- we don't typically test diagnostic 
behavior from codegen tests unless the diagnostic is only emitted during 
codegen.



Comment at: clang/test/Sema/m68k-mrtd.c:4-9
+#ifdef MRTD
+// expected-error@+3 {{function with no prototype cannot use the m68k_rtd 
calling convention}}
+#endif
+void foo(int arg) {
+  bar(arg);
+}

A better way to do this is to use `-verify=mrtd` on the line enabling rtd, and 
using `// rtd-error {{whatever}}` on the line being diagnosed. (Same comment 
applies throughout the file.)

Huh, I was unaware that implicit function declarations are using something 
other than the default calling convention (which is C, not m68k_rtd). Is this 
intentional?



Comment at: clang/test/Sema/m68k-mrtd.c:45
+extern void (*d)(int, ...);
+__attribute__((m68k_rtd)) extern void (*d)(int, ...);

Missing tests for:

* Function without a prototype
* Applying the attribute to a non-function
* Providing arguments to the attribute
* What should happen for C++ and things like member functions?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149867

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D149867: [Clang][M68k] Add Clang support for the new M68k_RTD CC

[Min-Yih Hsu via Phabricator via cfe-commits]

myhsu marked 3 inline comments as done.
myhsu added inline comments.



Comment at: clang/test/CodeGen/mrtd.c:4
 
-// CHECK: mrtd.c:10:3: warning: function with no prototype cannot use the 
stdcall calling convention
+// CHECK: mrtd.c:13:3: warning: function with no prototype cannot use the 
stdcall calling convention
 

jrtc27 wrote:
> Ew... this should be using -verify and `// expected-warning {{...}}`, then 
> the line number is relative to the comment's location. Or, really, it should 
> be in the Sema test...
> 
> Plus is it correct to call this stdcall on m68k? The GCC manpage only 
> mentions it in the x86 option description, not the m68k one.
Now this check is moved to `test/Sema/m68k-mrtd.c`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149867

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D149867: [Clang][M68k] Add Clang support for the new M68k_RTD CC

[Min-Yih Hsu via Phabricator via cfe-commits]

myhsu updated this revision to Diff 530089.
myhsu retitled this revision from "[M68k] Add Clang support for the new 
M68k_RTD CC" to "[Clang][M68k] Add Clang support for the new M68k_RTD CC".
myhsu edited the summary of this revision.
myhsu set the repository for this revision to rG LLVM Github Monorepo.
myhsu added a comment.
Herald added a subscriber: arphaman.
Herald added a reviewer: aaron.ballman.

Use a new CC, `CC_M68kRTD`, to model `llvm::CallingConv::M68k_RTD` instead of 
using X86StdCall


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149867

Files:
  clang/include/clang-c/Index.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/Specifiers.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/Type.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/Basic/Targets/M68k.cpp
  clang/lib/Basic/Targets/M68k.h
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/CodeGen/mrtd.c
  clang/test/Sema/m68k-mrtd.c
  clang/tools/libclang/CXType.cpp

Index: clang/tools/libclang/CXType.cpp
===
--- clang/tools/libclang/CXType.cpp
+++ clang/tools/libclang/CXType.cpp
@@ -678,6 +678,7 @@
   TCALLINGCONV(SwiftAsync);
   TCALLINGCONV(PreserveMost);
   TCALLINGCONV(PreserveAll);
+  TCALLINGCONV(M68kRTD);
 case CC_SpirFunction: return CXCallingConv_Unexposed;
 case CC_AMDGPUKernelCall: return CXCallingConv_Unexposed;
 case CC_OpenCLKernel: return CXCallingConv_Unexposed;
Index: clang/test/Sema/m68k-mrtd.c
===
--- /dev/null
+++ clang/test/Sema/m68k-mrtd.c
@@ -0,0 +1,45 @@
+// RUN: %clang_cc1 -DMRTD -mrtd -triple m68k-unknown-unknown -std=c89 -verify %s
+// RUN: %clang_cc1 -triple m68k-unknown-unknown -std=c89 -verify %s
+
+#ifdef MRTD
+// expected-error@+3 {{function with no prototype cannot use the m68k_rtd calling convention}}
+#endif
+void foo(int arg) {
+  bar(arg);
+}
+
+#ifndef MRTD
+// expected-note@+5 {{previous declaration is here}}
+// expected-error@+5 {{function declared 'm68k_rtd' here was previously declared without calling convention}}
+// expected-note@+5 {{previous declaration is here}}
+// expected-error@+5 {{function declared 'm68k_rtd' here was previously declared without calling convention}}
+#endif
+void nonvariadic1(int a, int b, int c);
+void __attribute__((m68k_rtd)) nonvariadic1(int a, int b, int c);
+void nonvariadic2(int a, int b, int c);
+void __attribute__((m68k_rtd)) nonvariadic2(int a, int b, int c) { }
+
+// expected-warning@+2 {{m68k_rtd calling convention is not supported on variadic function}}
+void variadic(int a, ...);
+void __attribute__((m68k_rtd)) variadic(int a, ...);
+
+#ifdef MRTD
+// expected-note@+3 {{previous declaration is here}}
+// expected-error@+3 {{redeclaration of 'a' with a different type: 'void ((*))(int, int) __attribute__((cdecl))' vs 'void (*)(int, int) __attribute__((m68k_rtd))'}}
+#endif
+extern void (*a)(int, int);
+__attribute__((cdecl)) extern void (*a)(int, int);
+
+extern void (*b)(int, ...);
+__attribute__((cdecl)) extern void (*b)(int, ...);
+
+#ifndef MRTD
+// expected-note@+3 {{previous declaration is here}}
+// expected-error@+3 {{redeclaration of 'c' with a different type: 'void ((*))(int, int) __attribute__((m68k_rtd))' vs 'void (*)(int, int)'}}
+#endif
+extern void (*c)(int, int);
+__attribute__((m68k_rtd)) extern void (*c)(int, int);
+
+// expected-warning@+2 {{m68k_rtd calling convention is not supported on variadic function}}
+extern void (*d)(int, ...);
+__attribute__((m68k_rtd)) extern void (*d)(int, ...);
Index: clang/test/CodeGen/mrtd.c
===
--- clang/test/CodeGen/mrtd.c
+++ clang/test/CodeGen/mrtd.c
@@ -1,20 +1,26 @@
-// RUN: %clang_cc1 -mrtd -triple i386-unknown-unknown -std=c89 -emit-llvm -o - %s 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -mrtd -triple i386-unknown-unknown -std=c89 -emit-llvm -o - %s 2>&1 | FileCheck --check-prefixes=CHECK,X86 %s
+// RUN: %clang_cc1 -mrtd -triple m68k-unknown-unknown -std=c89 -emit-llvm -o - %s 2>&1 | FileCheck --check-prefixes=CHECK,M68k %s
 
-// CHECK: mrtd.c:10:3: warning: function with no prototype cannot use the stdcall calling convention
+// X86: mrtd.c:13:3: warning: function with no prototype cannot use the stdcall calling convention
 
 void baz(int arg);
 
-// CHECK: define{{.*}} x86_stdcallcc void @foo(i32 noundef %arg) [[NUW:#[0-9]+]]
+// X86: define{{.*}} x86_stdcallcc void @foo(i32 noundef %arg) [[NUW:#[0-9]+]]
+// M68k: define{{.*}} m68k_rtdcc void @foo(i32 noundef %arg)
 void foo(int arg) {
-// CHECK: call x86_stdcallcc i32 @bar(
+// X86: call x86_stdcallcc i32 @bar(
+#ifndef mc68000