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



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

Reply via email to