[PATCH] D126479: [Clang] Allow 'Complex float __attribute__((mode(HC)))'

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a reviewer: tahonermann.
aaron.ballman added inline comments.


================
Comment at: clang/lib/Basic/TargetInfo.cpp:287-288
                                              FloatModeKind ExplicitType) const {
+  if (getHalfWidth() == BitWidth)
+    return FloatModeKind::Half;
   if (getFloatWidth() == BitWidth)
----------------
I *think* this is correct, but it's a bit worrying because we have multiple floating-point types with the same width. e.g., `HalfTy` and `BFloat16Ty`, so I am a bit uncomfortable with the `getRealTypeByWidth()` interface in general once we go down this route. `getRealTypeForBitwidth()` will have similar issues.


================
Comment at: clang/test/CodeGen/aarch64-attr-mode-complex.c:3-4
+
+typedef _Complex float c16a __attribute((mode(HC)));
+typedef _Complex double c16b __attribute((mode(HC)));
+typedef _Complex float c32a __attribute((mode(SC)));
----------------
Don't your changes also impact the behavior for `mode(HF)`? If so, there should be test coverage added for it.


================
Comment at: clang/test/Sema/attr-mode.c:40
 
+typedef _Complex float c16a __attribute((mode(HC)));
+int c16a_test[sizeof(c16a) == 4 ? 1 : -1];
----------------
jolanta.jensen wrote:
> mgabka wrote:
> > shouldn't we have here and in the line below : "// expected-no-diagnostics" ?
> I don't think we can have both expected-error and expected-no-diagnostic in the same file.
Nope -- that marker is only for when the entire test file expects no diagnostics.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126479