[PATCH] D97318: [clang][CodeGen] Allow fp16 arg pass by register

pengfei added a comment. In D97318#2587866 , @yaxunl wrote: > So should we revive this patch by classifying _Float16 as SSE, then it will > conform to the ABI. And we should be compatible with gcc assuming it also > follows the ABI. @pengfei Thanks

[PATCH] D97318: [clang][CodeGen] Allow fp16 arg pass by register

yaxunl marked an inline comment as done. yaxunl added a comment. So should we revive this patch by classifying _Float16 as SSE, then it will conform to the ABI. And we should be compatible with gcc assuming it also follows the ABI. @pengfei CHANGES SINCE LAST ACTION

[PATCH] D97318: [clang][CodeGen] Allow fp16 arg pass by register

hliao added inline comments. Comment at: clang/lib/CodeGen/TargetInfo.cpp:2821 +} else if (k == BuiltinType::Float16 || k == BuiltinType::Half) { + // AMD64 does not support operations on _Float16 or __fp16 other than + // load and store. For load/store operations,

[PATCH] D97318: [clang][CodeGen] Allow fp16 arg pass by register

hliao added inline comments. Comment at: clang/lib/CodeGen/TargetInfo.cpp:2821 +} else if (k == BuiltinType::Float16 || k == BuiltinType::Half) { + // AMD64 does not support operations on _Float16 or __fp16 other than + // load and store. For load/store operations,

[PATCH] D97318: [clang][CodeGen] Allow fp16 arg pass by register

yaxunl abandoned this revision. yaxunl marked an inline comment as done. yaxunl added inline comments. Comment at: clang/lib/CodeGen/TargetInfo.cpp:2824 + // which is equivalent to 16 bit integer. We need this to interop with + // gcc where 16 bit integer is used in

[PATCH] D97318: [clang][CodeGen] Allow fp16 arg pass by register

pengfei added a comment. I don't think the patch is doing right. Comment at: clang/lib/CodeGen/TargetInfo.cpp:2821 +} else if (k == BuiltinType::Float16 || k == BuiltinType::Half) { + // AMD64 does not support operations on _Float16 or __fp16 other than + // load

[PATCH] D97318: [clang][CodeGen] Allow fp16 arg pass by register

yaxunl updated this revision to Diff 325972. yaxunl marked 2 inline comments as done. yaxunl edited the summary of this revision. yaxunl added a comment. revised comments and fixed test CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97318/new/ https://reviews.llvm.org/D97318 Files:

[PATCH] D97318: [clang][CodeGen] Allow fp16 arg pass by register

yaxunl marked 3 inline comments as done. yaxunl added inline comments. Comment at: clang/lib/CodeGen/TargetInfo.cpp:2821 +} else if (k == BuiltinType::Float16 || k == BuiltinType::Half) { + // AMD64 does not support _Float16 or __fp16. When used in + // languages

[PATCH] D97318: [clang][CodeGen] Allow fp16 arg pass by register

pengfei requested changes to this revision. pengfei added inline comments. This revision now requires changes to proceed. Comment at: clang/lib/CodeGen/TargetInfo.cpp:2821 +} else if (k == BuiltinType::Float16 || k == BuiltinType::Half) { + // AMD64 does not support

[PATCH] D97318: [clang][CodeGen] Allow fp16 arg pass by register

yaxunl marked an inline comment as done. yaxunl added inline comments. Comment at: clang/lib/CodeGen/TargetInfo.cpp:2825 + // gcc where 16 bit integer is used in place of _Float16 or __fp16. + Lo = Integer; } tra wrote: > Do we need to set `Hi`,

[PATCH] D97318: [clang][CodeGen] Allow fp16 arg pass by register

tra added inline comments. Comment at: clang/lib/CodeGen/TargetInfo.cpp:2825 + // gcc where 16 bit integer is used in place of _Float16 or __fp16. + Lo = Integer; } Do we need to set `Hi`, too? We do set it for `int128`. CHANGES SINCE LAST

[PATCH] D97318: [clang][CodeGen] Allow fp16 arg pass by register

yaxunl created this revision. yaxunl added a reviewer: rjmccall. Herald added a subscriber: pengfei. yaxunl requested review of this revision. HIP supports _Float16 and __fp16 types. In x86_64 host they are for storage only. Since they have the same size and alignment as int16, they are supposed