[PATCH] D58930: Add XCOFF triple object format type for AIX

[Andrew Paprocki via Phabricator via cfe-commits]

apaprocki added inline comments.



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp:2079
+  if (log)
+log->Printf("sorry: unimplemented for XCOFF");
+  return false;

No need to be `sorry:` :) This should probably just say `error: XCOFF is 
unimplemented` to be more direct in case anything is expecting "error:" in the 
output.



Comment at: llvm/lib/Support/Triple.cpp:537
   return StringSwitch(EnvironmentName)
+// FIXME: We have to put XCOFF before COFF;
+// perhaps an order-independent pattern matching is desired?

hubert.reinterpretcast wrote:
> If the conclusion is that checking XCOFF before COFF is fine, then we should 
> remove the FIXME and just leave a normal comment.
Agreed, existing code seems fine as long as there is a comment explaining that 
`xcoff` must come before `coff` in case it isn't obvious at a quick glance.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58930



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

[PATCH] D59048: Add AIX Target Info

[Andrew Paprocki via Phabricator via cfe-commits]

apaprocki added inline comments.



Comment at: clang/lib/Basic/Targets/OSTargets.h:626
+
+// FIXME: Define AIX OS-Version Macros
+Builder.defineMacro("_AIX");

hubert.reinterpretcast wrote:
> Comments should be full sentences (with periods). Please update throughout 
> this patch.
I do not think it is realistic to support an AIX target prior to 7.1, so should 
this unconditionally define up to and including `_AIX71` similar to D18360 did 
(it was written earlier and defined up to `_AIX61` unconditionally) and leave a 
`FIXME` to determine when to emit `_AIX72` or any other later versions?



Comment at: clang/test/Preprocessor/init.c:6420
+// PPC64-AIX:#define _LONG_LONG 1
+// PPC64-AIX:#define _POWER 1
+// PPC64-AIX:#define __64BIT__ 1

XL on AIX emits `#define _LP64 1` in 64-bit mode and `#define _ILP32 1` in 
32-bit mode in the pre-defined macros. Is that important to capture?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59048



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

[PATCH] D18360: Add AIX Target/ToolChain to Clang Driver

[Andrew Paprocki via Phabricator via cfe-commits]

apaprocki added a comment.

@hubert.reinterpretcast Yes, this patch is available under the new license.


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

https://reviews.llvm.org/D18360



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