[PATCH] D125418: [Arm64EC 6/?] Implement C/C++ mangling for Arm64EC function definitions.

[chenglin.bi via Phabricator via cfe-commits]

bcl5980 added a comment.

Another thing we need consider here is this case:

  #pragma pack(push, 1)
  struct b64 {
  char a[64];
  };
  #pragma pack(pop)
  
  typedef b64 (fptrtype)(int a);
  
  b64 f(void* p, int a) {
  return ((fptrtype*)p)(a);
  }

For now we generate exit_thunk with type void f(void* sret(b64) ret, int a)

  $iexit_thunk$cdecl$v$i8i8:  // @"$iexit_thunk$cdecl$v$i8i8"
  .seh_proc $iexit_thunk$cdecl$v$i8i8
  // %bb.0:
sub sp, sp, #48
.seh_stackalloc 48
stp x29, x30, [sp, #32] // 16-byte Folded Spill
.seh_save_fplr  32
add x29, sp, #32
.seh_add_fp 32
.seh_endprologue
mov w1, w0
mov x0, x8
adrpx8, __os_arm64x_dispatch_call_no_redirect
ldr x8, [x8, :lo12:__os_arm64x_dispatch_call_no_redirect]
blr x8
.seh_startepilogue
ldp x29, x30, [sp, #32] // 16-byte Folded Reload
.seh_save_fplr  32
add sp, sp, #48
.seh_stackalloc 48
.seh_endepilogue
ret
.seh_endfunclet
.seh_endproc
  // -- End function
.globl  f
.deff;
.scl2;
.type   32;
.endef

But it looks Microsoft generate exit thunk with type void* f(int a)

  |$iexit_thunk$cdecl$i8$i8| PROC
  |$LN2|
pacibsp
stp fp,lr,[sp,#-0x10]!
mov fp,sp
sub sp,sp,#0x20
adrpx8,__os_arm64x_dispatch_call_no_redirect
ldr xip0,[x8,__os_arm64x_dispatch_call_no_redirect]
blr xip0
mov x0,x8
add sp,sp,#0x20
ldp fp,lr,[sp],#0x10
autibsp
ret
  
ENDP  ; |$iexit_thunk$cdecl$i8$i8|

But based on clang x86 on Windows, we also generate the function type with void 
f(void* sret(b64) ret, int a).
It looks clang is different from MSVC even in x86 ABI.
Do we need to follow MSVC to generate $iexit_thunk$cdecl$i8$i8 ? Or just follow 
clang's ABI and ignore the difference?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125418

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

[PATCH] D125418: [Arm64EC 6/?] Implement C/C++ mangling for Arm64EC function definitions.

[chenglin.bi via Phabricator via cfe-commits]

bcl5980 added a comment.

In D125418#3756223 , @efriedma wrote:

> There's no way the calling convention can change based on whether you're 
> calling a function vs. a function pointer.  I can't explain why MSVC is 
> generating different code.  I think we should just ignore it, at least for 
> now.

It's OK for me to ignore the difference but I think the main thing is not 
function or function pointer. It's how to generate the exit thunkwhen return 
with structure size value > 16.
https://godbolt.org/z/MWv4YaKdK
Three different way to call extern function, with three kind of exit thunks. 
All of them are keep the return value, not move the return value' point to the 
first argument.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125418

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

[PATCH] D125418: [Arm64EC 6/?] Implement C/C++ mangling for Arm64EC function definitions.

[chenglin.bi via Phabricator via cfe-commits]

bcl5980 added a comment.

In D125418#3759174 , @efriedma wrote:

> The reason struct returns require register shuffling is that AArch64 passes 
> the sret pointer in x8 (i.e. RAX), but the x64 calling convention expects in 
> in RCX (i.e. x0).

So, for the function: s64 f(int a):
AArch64 CC:void f(x8, x0)
X64 CC:void f(rcx[x0], rdx[x1])
AArch64 --> X64 we need to add instructions before blr

  mov x1, x0
  mov x0, x8

It can match `iexit_thunk$cdecl$m64$i8` when we call extern function not a 
function pointer.

> Have you tried to see if the Microsoft-generated thunk actually works?  I 
> found at least one bug in MSVC thunk generation and reported it to Microsoft. 
>  (Microsoft didn't acknowledge the report, but that's a different story...)

You are right. For now, I haven't tested too much case runtime. But it looks if 
a DLL import function pass to a function pointer, then call it will cause 
access violation.
Based on the debug result, it should be exit thunk issue, MSVC generate wrong 
thunk type.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125418

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

[PATCH] D125418: [Arm64EC 6/?] Implement C/C++ mangling for Arm64EC function definitions.

[chenglin.bi via Phabricator via cfe-commits]

bcl5980 added inline comments.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:5128
+// to the function itself; it points to a stub for the compiler.
+// FIXME: We also need to emit an entry thunk.
+SmallString<256> MangledName;

efriedma wrote:
> bcl5980 wrote:
> > A headache thing here.
> > We need to get the function definition with triple x64 to define entry 
> > thunk. For now the function definition here is aarch64 version.
> > For example the case in Microsoft doc "Understanding Arm64EC ABI and 
> > assembly code":
> > 
> > ```
> > struct SC {
> > char a;
> > char b;
> > char c;
> > };
> > int fB(int a, double b, int i1, int i2, int i3);
> > int fC(int a, struct SC c, int i1, int i2, int i3);
> > int fA(int a, double b, struct SC c, int i1, int i2, int i3) {
> > return fB(a, b, i1, i2, i3) + fC(a, c, i1, i2, i3);
> > }
> > ```
> > 
> > x64 version IR for fA is:
> > ```
> > define dso_local i32 @fA(i32 noundef %a, double noundef %b, ptr nocapture 
> > noundef readonly %c, i32 noundef %i1, i32 noundef %i2, i32 noundef %i3) 
> > local_unnamed_addr #0 { ... }
> > ```
> > aarch64 version IR for fA is:
> > 
> > ```
> > define dso_local i32 @"#fA"(i32 noundef %a, double noundef %b, i64 
> > %c.coerce, i32 noundef %i1, i32 noundef %i2, i32 noundef %i3) #0 {...}
> > ```
> > Arm64 will allow any size structure to be assigned to a register directly. 
> > x64 only allows sizes 1, 2, 4 and 8. 
> > Entry thunk follow x64 version function type. But we only have aarch64 
> > version function type.
> > 
> > I think the best way to do is create a x64 version codeGenModule and use 
> > the x64 CGM to generate the function type for entry thunk. But it is hard 
> > for me to do here. I tried a little but a lot of issues happen.
> > 
> > One other way is only modify `AArch64ABIInfo::classifyArgumentType`, copy 
> > the x64 code into the function and add a flag to determine which version 
> > will the function use. It is easier but I'm not sure it is the only 
> > difference between x64 and aarch64. Maybe the classify return also need to 
> > do this. And it is not a clean way I think.
> Oh, that's annoying... I hadn't considered the case of a struct of size 
> 3/5/6/7.
> 
> Like I noted on D126811, attaching thunks to calls is tricky if we try to do 
> it from clang.
> 
> Computing the right IR type shouldn't be that hard by itself; we can call 
> into call lowering code in TargetInfo without modifying much else.  (We just 
> need a bit to tell the TargetInfo to redirect the call, like D125419.  Use an 
> entry point like CodeGenTypes::arrangeCall.)  You don't need to mess with the 
> type system or anything like that.
> 
> The problem is correctly representing the lowered call in IR; we really don't 
> want to do lowering early because it will block optimizations.  I considered 
> using an operand bundle; we can probably make that work, but it's 
> complicated, and probably disables some optimizations.
> 
> I think the best thing we can do here is add an IR attribute to mark 
> arguments which are passed directly on AArch64, but need to be passed 
> indirectly for the x64 ABI.  Then AArch64Arm64ECCallLowering can check for 
> the attribute and modify its behavior.  This isn't really clean in the sense 
> that it's specific to the x64/aarch64 pair of calling conventions, but I 
> think the alternative is worse.
It looks not only 3/5/6/7, but also all size exclusive larger than 8 and less 
than 16 are difference between x86 ABI and Aarch64 ABI.
Maybe we can emit a function declaration here for the x86ABI thunk, then define 
it in Arm64ECCallLowering.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125418

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

[PATCH] D125418: [Arm64EC 6/?] Implement C/C++ mangling for Arm64EC function definitions.

[chenglin.bi via Phabricator via cfe-commits]

bcl5980 added inline comments.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:5128
+// to the function itself; it points to a stub for the compiler.
+// FIXME: We also need to emit an entry thunk.
+SmallString<256> MangledName;

efriedma wrote:
> bcl5980 wrote:
> > efriedma wrote:
> > > bcl5980 wrote:
> > > > A headache thing here.
> > > > We need to get the function definition with triple x64 to define entry 
> > > > thunk. For now the function definition here is aarch64 version.
> > > > For example the case in Microsoft doc "Understanding Arm64EC ABI and 
> > > > assembly code":
> > > > 
> > > > ```
> > > > struct SC {
> > > > char a;
> > > > char b;
> > > > char c;
> > > > };
> > > > int fB(int a, double b, int i1, int i2, int i3);
> > > > int fC(int a, struct SC c, int i1, int i2, int i3);
> > > > int fA(int a, double b, struct SC c, int i1, int i2, int i3) {
> > > > return fB(a, b, i1, i2, i3) + fC(a, c, i1, i2, i3);
> > > > }
> > > > ```
> > > > 
> > > > x64 version IR for fA is:
> > > > ```
> > > > define dso_local i32 @fA(i32 noundef %a, double noundef %b, ptr 
> > > > nocapture noundef readonly %c, i32 noundef %i1, i32 noundef %i2, i32 
> > > > noundef %i3) local_unnamed_addr #0 { ... }
> > > > ```
> > > > aarch64 version IR for fA is:
> > > > 
> > > > ```
> > > > define dso_local i32 @"#fA"(i32 noundef %a, double noundef %b, i64 
> > > > %c.coerce, i32 noundef %i1, i32 noundef %i2, i32 noundef %i3) #0 {...}
> > > > ```
> > > > Arm64 will allow any size structure to be assigned to a register 
> > > > directly. x64 only allows sizes 1, 2, 4 and 8. 
> > > > Entry thunk follow x64 version function type. But we only have aarch64 
> > > > version function type.
> > > > 
> > > > I think the best way to do is create a x64 version codeGenModule and 
> > > > use the x64 CGM to generate the function type for entry thunk. But it 
> > > > is hard for me to do here. I tried a little but a lot of issues happen.
> > > > 
> > > > One other way is only modify `AArch64ABIInfo::classifyArgumentType`, 
> > > > copy the x64 code into the function and add a flag to determine which 
> > > > version will the function use. It is easier but I'm not sure it is the 
> > > > only difference between x64 and aarch64. Maybe the classify return also 
> > > > need to do this. And it is not a clean way I think.
> > > Oh, that's annoying... I hadn't considered the case of a struct of size 
> > > 3/5/6/7.
> > > 
> > > Like I noted on D126811, attaching thunks to calls is tricky if we try to 
> > > do it from clang.
> > > 
> > > Computing the right IR type shouldn't be that hard by itself; we can call 
> > > into call lowering code in TargetInfo without modifying much else.  (We 
> > > just need a bit to tell the TargetInfo to redirect the call, like 
> > > D125419.  Use an entry point like CodeGenTypes::arrangeCall.)  You don't 
> > > need to mess with the type system or anything like that.
> > > 
> > > The problem is correctly representing the lowered call in IR; we really 
> > > don't want to do lowering early because it will block optimizations.  I 
> > > considered using an operand bundle; we can probably make that work, but 
> > > it's complicated, and probably disables some optimizations.
> > > 
> > > I think the best thing we can do here is add an IR attribute to mark 
> > > arguments which are passed directly on AArch64, but need to be passed 
> > > indirectly for the x64 ABI.  Then AArch64Arm64ECCallLowering can check 
> > > for the attribute and modify its behavior.  This isn't really clean in 
> > > the sense that it's specific to the x64/aarch64 pair of calling 
> > > conventions, but I think the alternative is worse.
> > It looks not only 3/5/6/7, but also all size exclusive larger than 8 and 
> > less than 16 are difference between x86 ABI and Aarch64 ABI.
> > Maybe we can emit a function declaration here for the x86ABI thunk, then 
> > define it in Arm64ECCallLowering.
> > 
> I think the sizes between 8 and 16 work correctly already?  All sizes greater 
> than 8 are passed indirectly on x86, and the thunk generation code accounts 
> for that.  But that's not really important for the general question.
> 
> We need to preserve the required semantics for both the AArch64 and x86 
> calling conventions.  There are basically the following possibilities:
> 
> - We compute the declaration of the thunk in the frontend, and attach it to 
> the call with an operand bundle.  Like I mentioned, I don't want to go down 
> this path: the operand bundle blocks optimizations, and it becomes more 
> complicated for other code to generate arm64ec compatible calls.
> - We don't compute the definition of the thunk in the frontend.  Given that, 
> the only other way to attach the information we need to the call is to use 
> attributes.  The simplest thing is probably to attach the attribute directly 
> to the argument; name it "arm64ec-thunk-pass-indirect", or something like 
>

[PATCH] D125418: [Arm64EC 6/?] Implement C/C++ mangling for Arm64EC function definitions.

[chenglin.bi via Phabricator via cfe-commits]

bcl5980 added inline comments.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:5128
+// to the function itself; it points to a stub for the compiler.
+// FIXME: We also need to emit an entry thunk.
+SmallString<256> MangledName;

A headache thing here.
We need to get the function definition with triple x64 to define entry thunk. 
For now the function definition here is aarch64 version.
For example the case in Microsoft doc "Understanding Arm64EC ABI and assembly 
code":

```
struct SC {
char a;
char b;
char c;
};
int fB(int a, double b, int i1, int i2, int i3);
int fC(int a, struct SC c, int i1, int i2, int i3);
int fA(int a, double b, struct SC c, int i1, int i2, int i3) {
return fB(a, b, i1, i2, i3) + fC(a, c, i1, i2, i3);
}
```

x64 version IR for fA is:
```
define dso_local i32 @fA(i32 noundef %a, double noundef %b, ptr nocapture 
noundef readonly %c, i32 noundef %i1, i32 noundef %i2, i32 noundef %i3) 
local_unnamed_addr #0 { ... }
```
aarch64 version IR for fA is:

```
define dso_local i32 @"#fA"(i32 noundef %a, double noundef %b, i64 %c.coerce, 
i32 noundef %i1, i32 noundef %i2, i32 noundef %i3) #0 {...}
```
Arm64 will allow any size structure to be assigned to a register directly. x64 
only allows sizes 1, 2, 4 and 8. 
Entry thunk follow x64 version function type. But we only have aarch64 version 
function type.

I think the best way to do is create a x64 version codeGenModule and use the 
x64 CGM to generate the function type for entry thunk. But it is hard for me to 
do here. I tried a little but a lot of issues happen.

One other way is only modify `AArch64ABIInfo::classifyArgumentType`, copy the 
x64 code into the function and add a flag to determine which version will the 
function use. It is easier but I'm not sure it is the only difference between 
x64 and aarch64. Maybe the classify return also need to do this. And it is not 
a clean way I think.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125418

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

[PATCH] D125418: [Arm64EC 6/?] Implement C/C++ mangling for Arm64EC function definitions.

[chenglin.bi via Phabricator via cfe-commits]

bcl5980 added inline comments.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:5128
+// to the function itself; it points to a stub for the compiler.
+// FIXME: We also need to emit an entry thunk.
+SmallString<256> MangledName;

efriedma wrote:
> bcl5980 wrote:
> > efriedma wrote:
> > > bcl5980 wrote:
> > > > efriedma wrote:
> > > > > bcl5980 wrote:
> > > > > > A headache thing here.
> > > > > > We need to get the function definition with triple x64 to define 
> > > > > > entry thunk. For now the function definition here is aarch64 
> > > > > > version.
> > > > > > For example the case in Microsoft doc "Understanding Arm64EC ABI 
> > > > > > and assembly code":
> > > > > > 
> > > > > > ```
> > > > > > struct SC {
> > > > > > char a;
> > > > > > char b;
> > > > > > char c;
> > > > > > };
> > > > > > int fB(int a, double b, int i1, int i2, int i3);
> > > > > > int fC(int a, struct SC c, int i1, int i2, int i3);
> > > > > > int fA(int a, double b, struct SC c, int i1, int i2, int i3) {
> > > > > > return fB(a, b, i1, i2, i3) + fC(a, c, i1, i2, i3);
> > > > > > }
> > > > > > ```
> > > > > > 
> > > > > > x64 version IR for fA is:
> > > > > > ```
> > > > > > define dso_local i32 @fA(i32 noundef %a, double noundef %b, ptr 
> > > > > > nocapture noundef readonly %c, i32 noundef %i1, i32 noundef %i2, 
> > > > > > i32 noundef %i3) local_unnamed_addr #0 { ... }
> > > > > > ```
> > > > > > aarch64 version IR for fA is:
> > > > > > 
> > > > > > ```
> > > > > > define dso_local i32 @"#fA"(i32 noundef %a, double noundef %b, i64 
> > > > > > %c.coerce, i32 noundef %i1, i32 noundef %i2, i32 noundef %i3) #0 
> > > > > > {...}
> > > > > > ```
> > > > > > Arm64 will allow any size structure to be assigned to a register 
> > > > > > directly. x64 only allows sizes 1, 2, 4 and 8. 
> > > > > > Entry thunk follow x64 version function type. But we only have 
> > > > > > aarch64 version function type.
> > > > > > 
> > > > > > I think the best way to do is create a x64 version codeGenModule 
> > > > > > and use the x64 CGM to generate the function type for entry thunk. 
> > > > > > But it is hard for me to do here. I tried a little but a lot of 
> > > > > > issues happen.
> > > > > > 
> > > > > > One other way is only modify 
> > > > > > `AArch64ABIInfo::classifyArgumentType`, copy the x64 code into the 
> > > > > > function and add a flag to determine which version will the 
> > > > > > function use. It is easier but I'm not sure it is the only 
> > > > > > difference between x64 and aarch64. Maybe the classify return also 
> > > > > > need to do this. And it is not a clean way I think.
> > > > > Oh, that's annoying... I hadn't considered the case of a struct of 
> > > > > size 3/5/6/7.
> > > > > 
> > > > > Like I noted on D126811, attaching thunks to calls is tricky if we 
> > > > > try to do it from clang.
> > > > > 
> > > > > Computing the right IR type shouldn't be that hard by itself; we can 
> > > > > call into call lowering code in TargetInfo without modifying much 
> > > > > else.  (We just need a bit to tell the TargetInfo to redirect the 
> > > > > call, like D125419.  Use an entry point like 
> > > > > CodeGenTypes::arrangeCall.)  You don't need to mess with the type 
> > > > > system or anything like that.
> > > > > 
> > > > > The problem is correctly representing the lowered call in IR; we 
> > > > > really don't want to do lowering early because it will block 
> > > > > optimizations.  I considered using an operand bundle; we can probably 
> > > > > make that work, but it's complicated, and probably disables some 
> > > > > optimizations.
> > > > > 
> > > > > I think the best thing we can do here is add an IR attribute to mark 
> > > > > arguments which are passed directly on AArch64, but need to be passed 
> > > > > indirectly for the x64 ABI.  Then AArch64Arm64ECCallLowering can 
> > > > > check for the attribute and modify its behavior.  This isn't really 
> > > > > clean in the sense that it's specific to the x64/aarch64 pair of 
> > > > > calling conventions, but I think the alternative is worse.
> > > > It looks not only 3/5/6/7, but also all size exclusive larger than 8 
> > > > and less than 16 are difference between x86 ABI and Aarch64 ABI.
> > > > Maybe we can emit a function declaration here for the x86ABI thunk, 
> > > > then define it in Arm64ECCallLowering.
> > > > 
> > > I think the sizes between 8 and 16 work correctly already?  All sizes 
> > > greater than 8 are passed indirectly on x86, and the thunk generation 
> > > code accounts for that.  But that's not really important for the general 
> > > question.
> > > 
> > > We need to preserve the required semantics for both the AArch64 and x86 
> > > calling conventions.  There are basically the following possibilities:
> > > 
> > > - We compute the declaration of the thunk in the frontend, and attach it 
> > > to the call with an operand bundle.  Like I mentioned, I don't want to go 
> > >

[PATCH] D125418: [Arm64EC 6/?] Implement C/C++ mangling for Arm64EC function definitions.

[chenglin.bi via Phabricator via cfe-commits]

bcl5980 added a comment.

A question about the mangle and alias part.
Should we move the code to create alias to backend also？Sometimes we will emit 
the alias here but later the function will be inlined or eliminated by DCE.
And later we need to emit alias for direct call thunk also, like 
$originname$exitthunk. Put all of them into arm64eccalllowering pass should be 
better I think.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125418

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

[PATCH] D125418: [Arm64EC 6/?] Implement C/C++ mangling for Arm64EC function definitions.

[chenglin.bi via Phabricator via cfe-commits]

bcl5980 added a comment.

In D125418#3806720 , @efriedma wrote:

>> Sometimes we will emit the alias here but later the function will be inlined 
>> or eliminated by DCE.
>
> If the alias is externally visible, it can't be eliminated; the compiler 
> can't tell whether the symbol is referenced.  If the alias isn't externally 
> visible, it's dead from the outset.  Not sure how this could become an issue.

There will be no functional issue here. I mean that we can avoid generate some 
redundant alias if it is in the arm64eccalllowering.

>> And later we need to emit alias for direct call thunk also, like 
>> $originname$exitthunk.
>
> Direct call thunks aren't directly relevant here; we only emit them for 
> declarations, not definitions.  I guess this does imply that we need to teach 
> arm64eccalllowering how to modify mangled symbol names... and we could use 
> that same code to insert the $$h.
>
>> Put all of them into arm64eccalllowering pass should be better I think.
>
> I really don't want to do demangling in arm64eccalllowering.  But looking at 
> the generated patterns a bit more closely, maybe we don't have to fully parse 
> the mangled symbol.  If we can get away with just parsing the "?symbolname@@" 
> at the beginning of the symbol, and ignore all the type-related stuff, I 
> guess that would be okay.
>
> Alternatively, I guess we could use attributes to communicate the different 
> mangled forms to the backend, but probably better to avoid that if we can.
>
> If we can solve the mangling issues, I guess generating the alias in 
> arm64eccalllowering would be fine.

As far as I know, there are three kinds of alias we need to generate, For 
example:

  extern "C" void function_name(void a)
  arm64 signature: #function_name(native)

If it is function definition, we need to create an alias to be x86 signature, 
and mangle a new name to arm64ec signature. That is what this change do.

  x86 signature: function_name

If it is a function direct call case, we need to create two alias:

  function thunk: #function_name$exit_thunk
  x86 signature: function_name

I don't understand too much why we need to demangle the function name in 
arm64eccallovering. It looks what we need to do is generate the arm64 signature 
name from the default symbol name which is x86 signature by default.
I'm not familiar with normal mangle rules. If `#` and `$$h` are unique, maybe 
we can just insert `#` on the beginning for C symbol name or insert `$$h` after 
first`@@` for C++ symbol name.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125418

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

[PATCH] D134788: [ARM64EC][clang-cl] Add /arm64EC flag

[chenglin.bi via Phabricator via cfe-commits]

bcl5980 updated this revision to Diff 463744.
bcl5980 added a comment.

add warning when /arm64EC has been override


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

https://reviews.llvm.org/D134788

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/cl-options.c


Index: clang/test/Driver/cl-options.c
===
--- clang/test/Driver/cl-options.c
+++ clang/test/Driver/cl-options.c
@@ -783,4 +783,10 @@
 // EXTERNAL_W0: "-Wno-system-headers"
 // EXTERNAL_Wn: "-Wsystem-headers"
 
+// RUN: %clang_cl -vctoolsdir "" /arm64EC /c -### -- %s 2>&1 | FileCheck %s 
--check-prefix ARM64EC
+// ARM64EC: "-triple" "arm64ec-pc-windows-msvc19.20.0"
+
+// RUN: %clang_cl -vctoolsdir "" /arm64EC /c -target x86_64-pc-windows-msvc  
-### -- %s 2>&1 | FileCheck %s --check-prefix ARM64EC_OVERRIDE
+// ARM64EC_OVERRIDE: warning: /arm64EC overrided by specifying 
target:x86_64-pc-windows-msvc; option ignored
+
 void f(void) { }
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -1252,6 +1252,8 @@
 T.setVendor(llvm::Triple::PC);
 T.setEnvironment(llvm::Triple::MSVC);
 T.setObjectFormat(llvm::Triple::COFF);
+if (Args.hasArg(options::OPT__SLASH_arm64EC))
+  T.setArch(llvm::Triple::aarch64, llvm::Triple::AArch64SubArch_arm64ec);
 TargetTriple = T.str();
   } else if (IsDXCMode()) {
 // Build TargetTriple from target_profile option for clang-dxc.
@@ -1376,6 +1378,15 @@
   const ToolChain &TC = getToolChain(
   *UArgs, computeTargetTriple(*this, TargetTriple, *UArgs));
 
+  // Report warning when arm64EC option is overrided by specified target
+  if (TC.getTriple().getArch() != llvm::Triple::aarch64 ||
+  TC.getTriple().getSubArch() != llvm::Triple::AArch64SubArch_arm64ec) {
+if (UArgs->hasArg(options::OPT__SLASH_arm64EC)) {
+  getDiags().Report(clang::diag::warn_target_override_arm64ec)
+  << TC.getTriple().str();
+}
+  }
+
   // The compilation takes ownership of Args.
   Compilation *C = new Compilation(*this, TC, UArgs.release(), TranslatedArgs,
ContainsError);
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -6695,6 +6695,8 @@
 def _SLASH_QIntel_jcc_erratum : CLFlag<"QIntel-jcc-erratum">,
   HelpText<"Align branches within 32-byte boundaries to mitigate the 
performance impact of the Intel JCC erratum.">,
   Alias;
+def _SLASH_arm64EC : CLFlag<"arm64EC">,
+  HelpText<"Set build target to arm64ec">;
 
 // Non-aliases:
 
Index: clang/include/clang/Basic/DiagnosticDriverKinds.td
===
--- clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -660,6 +660,10 @@
   "-fjmc works only for ELF; option ignored">,
   InGroup;
 
+def warn_target_override_arm64ec : Warning<
+  "/arm64EC overrided by specifying target:%0; option ignored">,
+  InGroup;
+
 def err_drv_target_variant_invalid : Error<
   "unsupported '%0' value '%1'; use 'ios-macabi' instead">;
 


Index: clang/test/Driver/cl-options.c
===
--- clang/test/Driver/cl-options.c
+++ clang/test/Driver/cl-options.c
@@ -783,4 +783,10 @@
 // EXTERNAL_W0: "-Wno-system-headers"
 // EXTERNAL_Wn: "-Wsystem-headers"
 
+// RUN: %clang_cl -vctoolsdir "" /arm64EC /c -### -- %s 2>&1 | FileCheck %s --check-prefix ARM64EC
+// ARM64EC: "-triple" "arm64ec-pc-windows-msvc19.20.0"
+
+// RUN: %clang_cl -vctoolsdir "" /arm64EC /c -target x86_64-pc-windows-msvc  -### -- %s 2>&1 | FileCheck %s --check-prefix ARM64EC_OVERRIDE
+// ARM64EC_OVERRIDE: warning: /arm64EC overrided by specifying target:x86_64-pc-windows-msvc; option ignored
+
 void f(void) { }
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -1252,6 +1252,8 @@
 T.setVendor(llvm::Triple::PC);
 T.setEnvironment(llvm::Triple::MSVC);
 T.setObjectFormat(llvm::Triple::COFF);
+if (Args.hasArg(options::OPT__SLASH_arm64EC))
+  T.setArch(llvm::Triple::aarch64, llvm::Triple::AArch64SubArch_arm64ec);
 TargetTriple = T.str();
   } else if (IsDXCMode()) {
 // Build TargetTriple from target_profile option for clang-dxc.
@@ -1376,6 +1378,15 @@
   const ToolChain &TC = getToolChain(
   *UArgs, computeTargetTriple(*this, TargetTriple, *UArgs));
 
+  // Report warning when arm64EC option is overrided by specified target
+  if (TC.getTriple().getArch() != llvm::Triple::aarch64 ||
+  TC.getTriple().getSubArch() != l

[PATCH] D134788: [ARM64EC][clang-cl] Add /arm64EC flag

[chenglin.bi via Phabricator via cfe-commits]

bcl5980 updated this revision to Diff 463746.

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

https://reviews.llvm.org/D134788

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/cl-options.c


Index: clang/test/Driver/cl-options.c
===
--- clang/test/Driver/cl-options.c
+++ clang/test/Driver/cl-options.c
@@ -783,4 +783,10 @@
 // EXTERNAL_W0: "-Wno-system-headers"
 // EXTERNAL_Wn: "-Wsystem-headers"
 
+// RUN: %clang_cl -vctoolsdir "" /arm64EC /c -### -- %s 2>&1 | FileCheck %s 
--check-prefix ARM64EC
+// ARM64EC: "-triple" "arm64ec-pc-windows-msvc19.20.0"
+
+// RUN: %clang_cl -vctoolsdir "" /arm64EC /c -target x86_64-pc-windows-msvc  
-### -- %s 2>&1 | FileCheck %s --check-prefix ARM64EC_OVERRIDE
+// ARM64EC_OVERRIDE: warning: /arm64EC has been overridden by specified 
target:x86_64-pc-windows-msvc; option ignored
+
 void f(void) { }
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -1252,6 +1252,8 @@
 T.setVendor(llvm::Triple::PC);
 T.setEnvironment(llvm::Triple::MSVC);
 T.setObjectFormat(llvm::Triple::COFF);
+if (Args.hasArg(options::OPT__SLASH_arm64EC))
+  T.setArch(llvm::Triple::aarch64, llvm::Triple::AArch64SubArch_arm64ec);
 TargetTriple = T.str();
   } else if (IsDXCMode()) {
 // Build TargetTriple from target_profile option for clang-dxc.
@@ -1376,6 +1378,15 @@
   const ToolChain &TC = getToolChain(
   *UArgs, computeTargetTriple(*this, TargetTriple, *UArgs));
 
+  // Report warning when arm64EC option is overridden by specified target
+  if (TC.getTriple().getArch() != llvm::Triple::aarch64 ||
+  TC.getTriple().getSubArch() != llvm::Triple::AArch64SubArch_arm64ec) {
+if (UArgs->hasArg(options::OPT__SLASH_arm64EC)) {
+  getDiags().Report(clang::diag::warn_target_override_arm64ec)
+  << TC.getTriple().str();
+}
+  }
+
   // The compilation takes ownership of Args.
   Compilation *C = new Compilation(*this, TC, UArgs.release(), TranslatedArgs,
ContainsError);
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -6695,6 +6695,8 @@
 def _SLASH_QIntel_jcc_erratum : CLFlag<"QIntel-jcc-erratum">,
   HelpText<"Align branches within 32-byte boundaries to mitigate the 
performance impact of the Intel JCC erratum.">,
   Alias;
+def _SLASH_arm64EC : CLFlag<"arm64EC">,
+  HelpText<"Set build target to arm64ec">;
 
 // Non-aliases:
 
Index: clang/include/clang/Basic/DiagnosticDriverKinds.td
===
--- clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -660,6 +660,10 @@
   "-fjmc works only for ELF; option ignored">,
   InGroup;
 
+def warn_target_override_arm64ec : Warning<
+  "/arm64EC has been overridden by specified target:%0; option ignored">,
+  InGroup;
+
 def err_drv_target_variant_invalid : Error<
   "unsupported '%0' value '%1'; use 'ios-macabi' instead">;
 


Index: clang/test/Driver/cl-options.c
===
--- clang/test/Driver/cl-options.c
+++ clang/test/Driver/cl-options.c
@@ -783,4 +783,10 @@
 // EXTERNAL_W0: "-Wno-system-headers"
 // EXTERNAL_Wn: "-Wsystem-headers"
 
+// RUN: %clang_cl -vctoolsdir "" /arm64EC /c -### -- %s 2>&1 | FileCheck %s --check-prefix ARM64EC
+// ARM64EC: "-triple" "arm64ec-pc-windows-msvc19.20.0"
+
+// RUN: %clang_cl -vctoolsdir "" /arm64EC /c -target x86_64-pc-windows-msvc  -### -- %s 2>&1 | FileCheck %s --check-prefix ARM64EC_OVERRIDE
+// ARM64EC_OVERRIDE: warning: /arm64EC has been overridden by specified target:x86_64-pc-windows-msvc; option ignored
+
 void f(void) { }
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -1252,6 +1252,8 @@
 T.setVendor(llvm::Triple::PC);
 T.setEnvironment(llvm::Triple::MSVC);
 T.setObjectFormat(llvm::Triple::COFF);
+if (Args.hasArg(options::OPT__SLASH_arm64EC))
+  T.setArch(llvm::Triple::aarch64, llvm::Triple::AArch64SubArch_arm64ec);
 TargetTriple = T.str();
   } else if (IsDXCMode()) {
 // Build TargetTriple from target_profile option for clang-dxc.
@@ -1376,6 +1378,15 @@
   const ToolChain &TC = getToolChain(
   *UArgs, computeTargetTriple(*this, TargetTriple, *UArgs));
 
+  // Report warning when arm64EC option is overridden by specified target
+  if (TC.getTriple().getArch() != llvm::Triple::aarch64 ||
+  TC.getTriple().getSubArch() != llvm::Triple::AArch64SubArch_arm64ec) {
+  

[PATCH] D134788: [ARM64EC][clang-cl] Add /arm64EC flag

[chenglin.bi via Phabricator via cfe-commits]

bcl5980 updated this revision to Diff 464128.
bcl5980 added a comment.

address comments.


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

https://reviews.llvm.org/D134788

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/cl-options.c


Index: clang/test/Driver/cl-options.c
===
--- clang/test/Driver/cl-options.c
+++ clang/test/Driver/cl-options.c
@@ -783,4 +783,8 @@
 // EXTERNAL_W0: "-Wno-system-headers"
 // EXTERNAL_Wn: "-Wsystem-headers"
 
+// RUN: %clang_cl -vctoolsdir "" /arm64EC /c -### -- %s 2>&1 | FileCheck %s 
--check-prefix ARM64EC
+// ARM64EC: "-triple" "arm64ec-pc-windows-msvc19.20.0"
+// ARM64EC-NOT: /arm64EC has been overridden by specified target
+
 void f(void) { }
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -1252,6 +1252,8 @@
 T.setVendor(llvm::Triple::PC);
 T.setEnvironment(llvm::Triple::MSVC);
 T.setObjectFormat(llvm::Triple::COFF);
+if (Args.hasArg(options::OPT__SLASH_arm64EC))
+  T.setArch(llvm::Triple::aarch64, llvm::Triple::AArch64SubArch_arm64ec);
 TargetTriple = T.str();
   } else if (IsDXCMode()) {
 // Build TargetTriple from target_profile option for clang-dxc.
@@ -1376,6 +1378,14 @@
   const ToolChain &TC = getToolChain(
   *UArgs, computeTargetTriple(*this, TargetTriple, *UArgs));
 
+  // Report warning when arm64EC option is overridden by specified target
+  if ((TC.getTriple().getArch() != llvm::Triple::aarch64 ||
+   TC.getTriple().getSubArch() != llvm::Triple::AArch64SubArch_arm64ec) &&
+  UArgs->hasArg(options::OPT__SLASH_arm64EC)) {
+getDiags().Report(clang::diag::warn_target_override_arm64ec)
+<< TC.getTriple().str();
+  }
+
   // The compilation takes ownership of Args.
   Compilation *C = new Compilation(*this, TC, UArgs.release(), TranslatedArgs,
ContainsError);
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -6695,6 +6695,8 @@
 def _SLASH_QIntel_jcc_erratum : CLFlag<"QIntel-jcc-erratum">,
   HelpText<"Align branches within 32-byte boundaries to mitigate the 
performance impact of the Intel JCC erratum.">,
   Alias;
+def _SLASH_arm64EC : CLFlag<"arm64EC">,
+  HelpText<"Set build target to arm64ec">;
 
 // Non-aliases:
 
Index: clang/include/clang/Basic/DiagnosticDriverKinds.td
===
--- clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -660,6 +660,10 @@
   "-fjmc works only for ELF; option ignored">,
   InGroup;
 
+def warn_target_override_arm64ec : Warning<
+  "/arm64EC has been overridden by specified target: %0; option ignored">,
+  InGroup;
+
 def err_drv_target_variant_invalid : Error<
   "unsupported '%0' value '%1'; use 'ios-macabi' instead">;
 


Index: clang/test/Driver/cl-options.c
===
--- clang/test/Driver/cl-options.c
+++ clang/test/Driver/cl-options.c
@@ -783,4 +783,8 @@
 // EXTERNAL_W0: "-Wno-system-headers"
 // EXTERNAL_Wn: "-Wsystem-headers"
 
+// RUN: %clang_cl -vctoolsdir "" /arm64EC /c -### -- %s 2>&1 | FileCheck %s --check-prefix ARM64EC
+// ARM64EC: "-triple" "arm64ec-pc-windows-msvc19.20.0"
+// ARM64EC-NOT: /arm64EC has been overridden by specified target
+
 void f(void) { }
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -1252,6 +1252,8 @@
 T.setVendor(llvm::Triple::PC);
 T.setEnvironment(llvm::Triple::MSVC);
 T.setObjectFormat(llvm::Triple::COFF);
+if (Args.hasArg(options::OPT__SLASH_arm64EC))
+  T.setArch(llvm::Triple::aarch64, llvm::Triple::AArch64SubArch_arm64ec);
 TargetTriple = T.str();
   } else if (IsDXCMode()) {
 // Build TargetTriple from target_profile option for clang-dxc.
@@ -1376,6 +1378,14 @@
   const ToolChain &TC = getToolChain(
   *UArgs, computeTargetTriple(*this, TargetTriple, *UArgs));
 
+  // Report warning when arm64EC option is overridden by specified target
+  if ((TC.getTriple().getArch() != llvm::Triple::aarch64 ||
+   TC.getTriple().getSubArch() != llvm::Triple::AArch64SubArch_arm64ec) &&
+  UArgs->hasArg(options::OPT__SLASH_arm64EC)) {
+getDiags().Report(clang::diag::warn_target_override_arm64ec)
+<< TC.getTriple().str();
+  }
+
   // The compilation takes ownership of Args.
   Compilation *C = new Compilation(*this, TC, UArgs.release(), TranslatedArgs,
ContainsError);
Index: clang/include/

[PATCH] D134788: [ARM64EC][clang-cl] Add /arm64EC flag

[chenglin.bi via Phabricator via cfe-commits]

bcl5980 added a comment.

In D134788#3823181 , @DavidSpickett 
wrote:

> Has Microsoft documented this option? Not doubting it exists but I mostly see 
> how to use it rather than a specific page describing the option and its 
> behaviour.
>
> Not a big deal but if there is one, please include a link to it in the commit 
> message.

For now, I haven't found any document for this option. I can update later when 
Microsoft publish the information.


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

https://reviews.llvm.org/D134788

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

[PATCH] D134788: [ARM64EC][clang-cl] Add /arm64EC flag

[chenglin.bi via Phabricator via cfe-commits]

bcl5980 added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:1384
+  TC.getTriple().getSubArch() != llvm::Triple::AArch64SubArch_arm64ec) {
+if (UArgs->hasArg(options::OPT__SLASH_arm64EC)) {
+  getDiags().Report(clang::diag::warn_target_override_arm64ec)

DavidSpickett wrote:
> DavidSpickett wrote:
> > This would read better to me if you put them all in one and put the most 
> > important check first like:
> > ```
> > if ( UArgs->hasArg(options::OPT__SLASH_arm64EC) &&
> > ( (TC.getTriple().getArch() != llvm::Triple::aarch64 ||
> >   TC.getTriple().getSubArch() != llvm::Triple::AArch64SubArch_arm64ec)) 
> > {
> > ```
> This is good but I didn't emphasise one bit. Putting the arm64ec option check 
> first saves reading the rest if the reader knows it's not relevant.
I believe the condition `UArgs->hasArg(options::OPT__SLASH_arm64EC)` should be 
much heavier than 
`(TC.getTriple().getArch() != llvm::Triple::aarch64 || 
TC.getTriple().getSubArch() != llvm::Triple::AArch64SubArch_arm64ec)`.
So I put the Arch and SubArch check first here. I don't understand why we 
should put the hasArg check first.


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

https://reviews.llvm.org/D134788

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

[PATCH] D134788: [ARM64EC][clang-cl] Add /arm64EC flag

[chenglin.bi via Phabricator via cfe-commits]

bcl5980 marked 4 inline comments as done.
bcl5980 added a comment.

Thanks for the detail explaination. I agree that if the cost is the same hasArg 
is better to move to the first.


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

https://reviews.llvm.org/D134788

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

[PATCH] D134788: [ARM64EC][clang-cl] Add /arm64EC flag

[chenglin.bi via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGb0fff3db6ada: [ARM64EC][clang-cl] Add /arm64EC flag 
(authored by bcl5980).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134788

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/cl-options.c


Index: clang/test/Driver/cl-options.c
===
--- clang/test/Driver/cl-options.c
+++ clang/test/Driver/cl-options.c
@@ -782,4 +782,8 @@
 // EXTERNAL_W0: "-Wno-system-headers"
 // EXTERNAL_Wn: "-Wsystem-headers"
 
+// RUN: %clang_cl -vctoolsdir "" /arm64EC /c -### -- %s 2>&1 | FileCheck %s 
--check-prefix ARM64EC
+// ARM64EC: "-triple" "arm64ec-pc-windows-msvc19.20.0"
+// ARM64EC-NOT: /arm64EC has been overridden by specified target
+
 void f(void) { }
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -1256,6 +1256,8 @@
 T.setVendor(llvm::Triple::PC);
 T.setEnvironment(llvm::Triple::MSVC);
 T.setObjectFormat(llvm::Triple::COFF);
+if (Args.hasArg(options::OPT__SLASH_arm64EC))
+  T.setArch(llvm::Triple::aarch64, llvm::Triple::AArch64SubArch_arm64ec);
 TargetTriple = T.str();
   } else if (IsDXCMode()) {
 // Build TargetTriple from target_profile option for clang-dxc.
@@ -1380,6 +1382,14 @@
   const ToolChain &TC = getToolChain(
   *UArgs, computeTargetTriple(*this, TargetTriple, *UArgs));
 
+  // Report warning when arm64EC option is overridden by specified target
+  if ((TC.getTriple().getArch() != llvm::Triple::aarch64 ||
+   TC.getTriple().getSubArch() != llvm::Triple::AArch64SubArch_arm64ec) &&
+  UArgs->hasArg(options::OPT__SLASH_arm64EC)) {
+getDiags().Report(clang::diag::warn_target_override_arm64ec)
+<< TC.getTriple().str();
+  }
+
   // The compilation takes ownership of Args.
   Compilation *C = new Compilation(*this, TC, UArgs.release(), TranslatedArgs,
ContainsError);
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -6695,6 +6695,8 @@
 def _SLASH_QIntel_jcc_erratum : CLFlag<"QIntel-jcc-erratum">,
   HelpText<"Align branches within 32-byte boundaries to mitigate the 
performance impact of the Intel JCC erratum.">,
   Alias;
+def _SLASH_arm64EC : CLFlag<"arm64EC">,
+  HelpText<"Set build target to arm64ec">;
 
 // Non-aliases:
 
Index: clang/include/clang/Basic/DiagnosticDriverKinds.td
===
--- clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -655,6 +655,10 @@
   "-fjmc works only for ELF; option ignored">,
   InGroup;
 
+def warn_target_override_arm64ec : Warning<
+  "/arm64EC has been overridden by specified target: %0; option ignored">,
+  InGroup;
+
 def err_drv_target_variant_invalid : Error<
   "unsupported '%0' value '%1'; use 'ios-macabi' instead">;
 


Index: clang/test/Driver/cl-options.c
===
--- clang/test/Driver/cl-options.c
+++ clang/test/Driver/cl-options.c
@@ -782,4 +782,8 @@
 // EXTERNAL_W0: "-Wno-system-headers"
 // EXTERNAL_Wn: "-Wsystem-headers"
 
+// RUN: %clang_cl -vctoolsdir "" /arm64EC /c -### -- %s 2>&1 | FileCheck %s --check-prefix ARM64EC
+// ARM64EC: "-triple" "arm64ec-pc-windows-msvc19.20.0"
+// ARM64EC-NOT: /arm64EC has been overridden by specified target
+
 void f(void) { }
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -1256,6 +1256,8 @@
 T.setVendor(llvm::Triple::PC);
 T.setEnvironment(llvm::Triple::MSVC);
 T.setObjectFormat(llvm::Triple::COFF);
+if (Args.hasArg(options::OPT__SLASH_arm64EC))
+  T.setArch(llvm::Triple::aarch64, llvm::Triple::AArch64SubArch_arm64ec);
 TargetTriple = T.str();
   } else if (IsDXCMode()) {
 // Build TargetTriple from target_profile option for clang-dxc.
@@ -1380,6 +1382,14 @@
   const ToolChain &TC = getToolChain(
   *UArgs, computeTargetTriple(*this, TargetTriple, *UArgs));
 
+  // Report warning when arm64EC option is overridden by specified target
+  if ((TC.getTriple().getArch() != llvm::Triple::aarch64 ||
+   TC.getTriple().getSubArch() != llvm::Triple::AArch64SubArch_arm64ec) &&
+  UArgs->hasArg(options::OPT__SLASH_arm64EC)) {
+getDiags().Report(clang::diag::warn_target_override_arm64ec)
+<< TC.getTriple().str();
+  }
+
   // The compilation takes ownership of Args.
   Compilation *C = new Compilation(

[PATCH] D134788: [ARM64EC][clang-cl] Add /arm64EC flag

[chenglin.bi via Phabricator via cfe-commits]

bcl5980 added a comment.

In D134788#3830987 , @efriedma wrote:

> Missing testcase for the case where the warning is triggered?

Based on David's comment I remove the test case with specified target.

  Not going to catch much in this change but if someone goes digging into 
target setting they might change it by other means than --target and not 
realise.

If you think that's necessary I can commit a NFC change to add it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134788

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

[PATCH] D134788: [ARM64EC][clang-cl] Add arm64EC test; NFC

[chenglin.bi via Phabricator via cfe-commits]

bcl5980 updated this revision to Diff 464909.
bcl5980 retitled this revision from "[ARM64EC][clang-cl] Add /arm64EC flag" to 
"[ARM64EC][clang-cl] Add arm64EC test; NFC".

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

https://reviews.llvm.org/D134788

Files:
  clang/test/Driver/cl-options.c


Index: clang/test/Driver/cl-options.c
===
--- clang/test/Driver/cl-options.c
+++ clang/test/Driver/cl-options.c
@@ -782,8 +782,11 @@
 // EXTERNAL_W0: "-Wno-system-headers"
 // EXTERNAL_Wn: "-Wsystem-headers"
 
-// RUN: %clang_cl -vctoolsdir "" /arm64EC /c -### -- %s 2>&1 | FileCheck %s 
--check-prefix ARM64EC
-// ARM64EC: "-triple" "arm64ec-pc-windows-msvc19.20.0"
+// RUN: %clang_cl -vctoolsdir "" /arm64EC /c -### -- %s 2>&1 | FileCheck 
--check-prefix=ARM64EC %s 
 // ARM64EC-NOT: /arm64EC has been overridden by specified target
+// ARM64EC: "-triple" "arm64ec-pc-windows-msvc19.20.0"
+
+// RUN: %clang_cl -vctoolsdir "" /arm64EC /c -target x86_64-pc-windows-msvc  
-### -- %s 2>&1 | FileCheck --check-prefix=ARM64EC_OVERRIDE %s
+// ARM64EC_OVERRIDE: warning: /arm64EC has been overridden by specified 
target: x86_64-pc-windows-msvc; option ignored
 
 void f(void) { }


Index: clang/test/Driver/cl-options.c
===
--- clang/test/Driver/cl-options.c
+++ clang/test/Driver/cl-options.c
@@ -782,8 +782,11 @@
 // EXTERNAL_W0: "-Wno-system-headers"
 // EXTERNAL_Wn: "-Wsystem-headers"
 
-// RUN: %clang_cl -vctoolsdir "" /arm64EC /c -### -- %s 2>&1 | FileCheck %s --check-prefix ARM64EC
-// ARM64EC: "-triple" "arm64ec-pc-windows-msvc19.20.0"
+// RUN: %clang_cl -vctoolsdir "" /arm64EC /c -### -- %s 2>&1 | FileCheck --check-prefix=ARM64EC %s 
 // ARM64EC-NOT: /arm64EC has been overridden by specified target
+// ARM64EC: "-triple" "arm64ec-pc-windows-msvc19.20.0"
+
+// RUN: %clang_cl -vctoolsdir "" /arm64EC /c -target x86_64-pc-windows-msvc  -### -- %s 2>&1 | FileCheck --check-prefix=ARM64EC_OVERRIDE %s
+// ARM64EC_OVERRIDE: warning: /arm64EC has been overridden by specified target: x86_64-pc-windows-msvc; option ignored
 
 void f(void) { }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits