[clang] [NFC][HLSL] Cleanup TargetInfo handling (PR #90694)

[Chris B via cfe-commits]

@@ -406,6 +406,16 @@ void TargetInfo::adjust(DiagnosticsEngine &Diags, 
LangOptions &Opts) {
 LongDoubleAlign = 64;
   }
 
+  // HLSL explicitly defines the sizes and formats of some data types, and we
+  // need to conform to those regardless of what architecture you are 
targeting.
+  if (Opts.HLSL) {
+LongWidth = LongAlign = 64;
+if (!Opts.NativeHalfType) {
+  HalfFormat = &llvm::APFloat::IEEEsingle();
+  HalfWidth = HalfAlign = 32;

llvm-beanz wrote:

Yes, and I totally glossed over that this fixes a bug. Without this line 
`sizeof(half)` returns the wrong value.

https://github.com/llvm/llvm-project/pull/90694
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [NFC][HLSL] Cleanup TargetInfo handling (PR #90694)

[Chris B via cfe-commits]

@@ -1612,15 +1612,7 @@ const llvm::fltSemantics 
&ASTContext::getFloatTypeSemantics(QualType T) const {
   case BuiltinType::Float16:
 return Target->getHalfFormat();
   case BuiltinType::Half:
-// For HLSL, when the native half type is disabled, half will be treat as
-// float.
-if (getLangOpts().HLSL)
-  if (getLangOpts().NativeHalfType)
-return Target->getHalfFormat();
-  else
-return Target->getFloatFormat();
-else
-  return Target->getHalfFormat();
+return Target->getHalfFormat();
   case BuiltinType::Float:  return Target->getFloatFormat();

llvm-beanz wrote:

Yea, this code would completely change with clang-format, but the bot only 
cares about the lines I changed. I tried not to include larger whitespace 
changes.

https://github.com/llvm/llvm-project/pull/90694
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [NFC][HLSL] Cleanup TargetInfo handling (PR #90694)

[Cooper Partin via cfe-commits]

https://github.com/coopp approved this pull request.

Just a small nit from me about switch( ) statement formatting in existing code. 
 Not the new code.  Looks good to me.

https://github.com/llvm/llvm-project/pull/90694
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [NFC][HLSL] Cleanup TargetInfo handling (PR #90694)

[Cooper Partin via cfe-commits]

@@ -1612,15 +1612,7 @@ const llvm::fltSemantics 
&ASTContext::getFloatTypeSemantics(QualType T) const {
   case BuiltinType::Float16:
 return Target->getHalfFormat();
   case BuiltinType::Half:
-// For HLSL, when the native half type is disabled, half will be treat as
-// float.
-if (getLangOpts().HLSL)
-  if (getLangOpts().NativeHalfType)
-return Target->getHalfFormat();
-  else
-return Target->getFloatFormat();
-else
-  return Target->getHalfFormat();
+return Target->getHalfFormat();
   case BuiltinType::Float:  return Target->getFloatFormat();

coopp wrote:

Existing (not in your new changes):  This switch statement is strangely 
formatted.  I seem some cases with the return on the same line as the case: and 
others on the next line.  Maybe this is how clang-format handles this?

Example:

case BuiltinType::Double: return Target->getDoubleFormat();
  case BuiltinType::Ibm128:
return Target->getIbm128Format();



https://github.com/llvm/llvm-project/pull/90694
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [NFC][HLSL] Cleanup TargetInfo handling (PR #90694)

[Farzon Lotfi via cfe-commits]

@@ -406,6 +406,16 @@ void TargetInfo::adjust(DiagnosticsEngine &Diags, 
LangOptions &Opts) {
 LongDoubleAlign = 64;
   }
 
+  // HLSL explicitly defines the sizes and formats of some data types, and we
+  // need to conform to those regardless of what architecture you are 
targeting.
+  if (Opts.HLSL) {
+LongWidth = LongAlign = 64;
+if (!Opts.NativeHalfType) {
+  HalfFormat = &llvm::APFloat::IEEEsingle();
+  HalfWidth = HalfAlign = 32;

farzonl wrote:

Is there a test case we can add for this?

https://github.com/llvm/llvm-project/pull/90694
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [NFC][HLSL] Cleanup TargetInfo handling (PR #90694)

[Farzon Lotfi via cfe-commits]

https://github.com/farzonl approved this pull request.


https://github.com/llvm/llvm-project/pull/90694
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [NFC][HLSL] Cleanup TargetInfo handling (PR #90694)

[Justin Bogner via cfe-commits]

https://github.com/bogner approved this pull request.


https://github.com/llvm/llvm-project/pull/90694
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [NFC][HLSL] Cleanup TargetInfo handling (PR #90694)

[Damyan Pepper via cfe-commits]

https://github.com/damyanp approved this pull request.


https://github.com/llvm/llvm-project/pull/90694
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [NFC][HLSL] Cleanup TargetInfo handling (PR #90694)

[Chris B via cfe-commits]

@@ -406,6 +406,16 @@ void TargetInfo::adjust(DiagnosticsEngine &Diags, 
LangOptions &Opts) {
 LongDoubleAlign = 64;
   }
 
+  // HLSL explicitly defines the sizes and formats of some data types, and we
+  // need to conform to those regardless of what architecture you are 
targeting.
+  if (Opts.HLSL) {
+LongWidth = LongAlign = 64;
+if (!Opts.NativeHalfType) {
+  HalfFormat = &llvm::APFloat::IEEEsingle();
+  HalfWidth = HalfAlign = 32;

llvm-beanz wrote:

Just as a note, we don't actually use this value anywhere in the HLSL code 
generation paths (yet), which is why it wasn't causing problems that we 
overrode the format returned by the AST context, but didn't override these 
values.

https://github.com/llvm/llvm-project/pull/90694
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [NFC][HLSL] Cleanup TargetInfo handling (PR #90694)

[via cfe-commits]

llvmbot wrote:




@llvm/pr-subscribers-clang

Author: Chris B (llvm-beanz)


Changes

We had some odd places where we set target behaviors. We were setting the long 
size in target-specific code, but it should be language-based. We were not 
setting the Half float type semantics correctly, and instead were overriding 
the query in the AST context.

This change is NFC, but it moves existing code to the right places in the 
Target so that as we continue working on target and language feature they are 
controlled in the right places.

---
Full diff: https://github.com/llvm/llvm-project/pull/90694.diff


3 Files Affected:

- (modified) clang/lib/AST/ASTContext.cpp (+1-9) 
- (modified) clang/lib/Basic/TargetInfo.cpp (+10) 
- (modified) clang/lib/Basic/Targets/DirectX.h (-1) 


``diff
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index cbf4932aff9a6b..565870eba7014f 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -1612,15 +1612,7 @@ const llvm::fltSemantics 
&ASTContext::getFloatTypeSemantics(QualType T) const {
   case BuiltinType::Float16:
 return Target->getHalfFormat();
   case BuiltinType::Half:
-// For HLSL, when the native half type is disabled, half will be treat as
-// float.
-if (getLangOpts().HLSL)
-  if (getLangOpts().NativeHalfType)
-return Target->getHalfFormat();
-  else
-return Target->getFloatFormat();
-else
-  return Target->getHalfFormat();
+return Target->getHalfFormat();
   case BuiltinType::Float:  return Target->getFloatFormat();
   case BuiltinType::Double: return Target->getDoubleFormat();
   case BuiltinType::Ibm128:
diff --git a/clang/lib/Basic/TargetInfo.cpp b/clang/lib/Basic/TargetInfo.cpp
index f96956f31d50dc..29f5cd14e46e11 100644
--- a/clang/lib/Basic/TargetInfo.cpp
+++ b/clang/lib/Basic/TargetInfo.cpp
@@ -406,6 +406,16 @@ void TargetInfo::adjust(DiagnosticsEngine &Diags, 
LangOptions &Opts) {
 LongDoubleAlign = 64;
   }
 
+  // HLSL explicitly defines the sizes and formats of some data types, and we
+  // need to conform to those regardless of what architecture you are 
targeting.
+  if (Opts.HLSL) {
+LongWidth = LongAlign = 64;
+if (!Opts.NativeHalfType) {
+  HalfFormat = &llvm::APFloat::IEEEsingle();
+  HalfWidth = HalfAlign = 32;
+}
+  }
+
   if (Opts.OpenCL) {
 // OpenCL C requires specific widths for types, irrespective of
 // what these normally are for the target.
diff --git a/clang/lib/Basic/Targets/DirectX.h 
b/clang/lib/Basic/Targets/DirectX.h
index acfcc8c47ba950..a084e2823453fc 100644
--- a/clang/lib/Basic/Targets/DirectX.h
+++ b/clang/lib/Basic/Targets/DirectX.h
@@ -53,7 +53,6 @@ class LLVM_LIBRARY_VISIBILITY DirectXTargetInfo : public 
TargetInfo {
   : TargetInfo(Triple) {
 TLSSupported = false;
 VLASupported = false;
-LongWidth = LongAlign = 64;
 AddrSpaceMap = &DirectXAddrSpaceMap;
 UseAddrSpaceMapMangling = true;
 HasLegalHalfType = true;

``




https://github.com/llvm/llvm-project/pull/90694
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [NFC][HLSL] Cleanup TargetInfo handling (PR #90694)

[Chris B via cfe-commits]

https://github.com/llvm-beanz created 
https://github.com/llvm/llvm-project/pull/90694

We had some odd places where we set target behaviors. We were setting the long 
size in target-specific code, but it should be language-based. We were not 
setting the Half float type semantics correctly, and instead were overriding 
the query in the AST context.

This change is NFC, but it moves existing code to the right places in the 
Target so that as we continue working on target and language feature they are 
controlled in the right places.

>From 2464bcb75b047c49076f0718470f27a561252a62 Mon Sep 17 00:00:00 2001
From: Chris Bieneman 
Date: Tue, 30 Apr 2024 20:49:35 -0500
Subject: [PATCH] [NFC][HLSL] Cleanup TargetInfo handling

We had some odd places where we set target behaviors. We were setting
the long size in target-specific code, but it should be language-based.
We were not setting the Half float type semantics correctly, and instead
were overriding the query in the AST context.

This change is NFC, but it moves existing code to the right places in
the Target so that as we continue working on target and language feature
they are controlled in the right places.
---
 clang/lib/AST/ASTContext.cpp  | 10 +-
 clang/lib/Basic/TargetInfo.cpp| 10 ++
 clang/lib/Basic/Targets/DirectX.h |  1 -
 3 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index cbf4932aff9a6b..565870eba7014f 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -1612,15 +1612,7 @@ const llvm::fltSemantics 
&ASTContext::getFloatTypeSemantics(QualType T) const {
   case BuiltinType::Float16:
 return Target->getHalfFormat();
   case BuiltinType::Half:
-// For HLSL, when the native half type is disabled, half will be treat as
-// float.
-if (getLangOpts().HLSL)
-  if (getLangOpts().NativeHalfType)
-return Target->getHalfFormat();
-  else
-return Target->getFloatFormat();
-else
-  return Target->getHalfFormat();
+return Target->getHalfFormat();
   case BuiltinType::Float:  return Target->getFloatFormat();
   case BuiltinType::Double: return Target->getDoubleFormat();
   case BuiltinType::Ibm128:
diff --git a/clang/lib/Basic/TargetInfo.cpp b/clang/lib/Basic/TargetInfo.cpp
index f96956f31d50dc..29f5cd14e46e11 100644
--- a/clang/lib/Basic/TargetInfo.cpp
+++ b/clang/lib/Basic/TargetInfo.cpp
@@ -406,6 +406,16 @@ void TargetInfo::adjust(DiagnosticsEngine &Diags, 
LangOptions &Opts) {
 LongDoubleAlign = 64;
   }
 
+  // HLSL explicitly defines the sizes and formats of some data types, and we
+  // need to conform to those regardless of what architecture you are 
targeting.
+  if (Opts.HLSL) {
+LongWidth = LongAlign = 64;
+if (!Opts.NativeHalfType) {
+  HalfFormat = &llvm::APFloat::IEEEsingle();
+  HalfWidth = HalfAlign = 32;
+}
+  }
+
   if (Opts.OpenCL) {
 // OpenCL C requires specific widths for types, irrespective of
 // what these normally are for the target.
diff --git a/clang/lib/Basic/Targets/DirectX.h 
b/clang/lib/Basic/Targets/DirectX.h
index acfcc8c47ba950..a084e2823453fc 100644
--- a/clang/lib/Basic/Targets/DirectX.h
+++ b/clang/lib/Basic/Targets/DirectX.h
@@ -53,7 +53,6 @@ class LLVM_LIBRARY_VISIBILITY DirectXTargetInfo : public 
TargetInfo {
   : TargetInfo(Triple) {
 TLSSupported = false;
 VLASupported = false;
-LongWidth = LongAlign = 64;
 AddrSpaceMap = &DirectXAddrSpaceMap;
 UseAddrSpaceMapMangling = true;
 HasLegalHalfType = true;

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