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

================
Comment at: clang/lib/Driver/ToolChains/AIX.cpp:45
+  // Acccept any mixture of instructions.
+  CmdArgs.push_back("-many");
+
----------------
Xiangling_L wrote:
> GCC invokes system assembler also with options `-mpwr4` and `-u`, I think you 
> need to verify that do we need those? And as far as I can recall, `-mpwr4` is 
> to pick up new version AIX instruction set, and `-u` is to suppress warning 
> for undefined symbols. 90% sure that we need `-mpwr4`(I could be wrong), but 
> not sure about `-u`.
For the `-u` option, I'll be adding it to this patch with a FIXME that we'd 
like to remove it in the future. The rationale is that, since currently we do 
not have the assembly writing ready that writes the extern(s), we'll pass in 
this `-u` to suppress any related warnings. Once the assembly writing is ready, 
we should remove this flag as it potentially covers up for undefined symbols 
that are actually problematic.

For the `-m` option, adding `-many` seemed to be more suitable at the moment. 
During code generation, the compiler should've already guaranteed that it used 
a correct and appropriate instruction set (i.e., conforms to the user specified 
driver option `-mcpu=`). The `-m` assembler option here double checks the same 
thing that's already been checked at codegen. That said, we can just pass in 
`-many` to accept all and rely on codegen to do the checking. On the other 
hand, potential problems might get away with it in one of the two scenarios I 
can think of right now,


  # User passes in a `.s` assembly file that uses instructions from the 
super-set of what's specified in `-mcpu=`. This is mostly on the user side 
because essentially they pass in contradictory inputs that are not going to fly.

  # User passes in a `.c` source file, but codegen hit some issue and falsely 
decides to use a super-set of what's specified in `-mcpu=`. Given that we don't 
have codegen ready yet, we don't know how reliable it might be. I would suggest 
that we keep `-many` as it is for now, and we may change it when needed once we 
are more clear on code generation. 


With regard to GCC's behaviour, GCC would append first `-mpwr4` then `-many` to 
the system assembler, which effectively means adding `-many` solely because the 
last `-m` flag would overwrite all its preceding one(s).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69620



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

Reply via email to