[PATCH] D64283: [PowerPC] Support -mabi=ieeelongdouble and -mabi=ibmlongdouble

Fangrui Song via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Jul 9 20:07:12 PDT 2019


MaskRay marked an inline comment as done.
MaskRay added inline comments.


================
Comment at: lib/Driver/ToolChains/Clang.cpp:1825
+      // that don't use the altivec abi.
+      if (!SeenOther)
+        ABIName = A->getValue();
----------------
hfinkel wrote:
> This seems like an unintentional behavior change on top of the existing behavior (which may have also been not quite right). As best I can tell, we're trying to set ABIName equal to the last ABI type specified, unless that type is "altivec", in which case the ABI name should be its default value (either a nullptr or something like "elfv2"). With this change, we'll now take the first ABI name seen, not the last (as we'll get SeenOther to true after the first name).
> 
> Maybe we should loop over the list multiple times, once to get the long-double format, once to get the basic ABI name (where we save the default, and then if the last name found is "altivec", then reset to the default)?
> 
My understanding (could be wrong) is that "ibmlongdouble", "ieeelongdouble", and "altivec" cannot be ABIName, so I write it this way (filtered_reverse + SeenLongDouble + SeenOther).

e.g. with -mabi=elfv2 -mabi=ieeelongdouble, ABIName should be "elfv2". With -mabi=elfv1 -mabi=ieeelongdouble, ABIName should be "elfv1" (no local entry). This matches the behavior I observed from powerpc64le-linux-gnu-gcc.

Before the change, ABIName is "ieeelongdouble". I think that is not desired.


Repository:
  rC Clang

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

https://reviews.llvm.org/D64283





More information about the cfe-commits mailing list