[PATCH] D36550: [mips] Notify user that `-mabicalls` is ignored on non-PIC N64 ABI

[Simon Dardis via Phabricator via cfe-commits]

sdardis added a comment.

Slight tweak to the summary:

> The -mabicalls option does not have a sense in case of non position independent code on N64 ABI. After this change driver starts to show a warning that -mabicalls is ignored in that case.

The -mabicalls option does not make sense in the case of non position independent code for the N64 ABI. After this change the driver shows a warning that -mabicalls is ignored in that case.

I think the major change in this patch is overly complex for what it should be, suggestion inline.



================
Comment at: include/clang/Basic/DiagnosticDriverKinds.td:297
+  "ignoring '-mabicalls' option as it cannot be used with "
+  "non position-independent code and N64 ABI">,
+  InGroup<OptionIgnored>;
----------------
and the N64 ABI


================
Comment at: lib/Driver/ToolChains/Arch/Mips.cpp:233-253
+  Arg *ABICallsArg =
+      Args.getLastArg(options::OPT_mabicalls, options::OPT_mno_abicalls);
+  if (!ABICallsArg)
+    NeedNoAbiCalls = DefNoAbiCalls;
+  else {
+    if (ABICallsArg->getOption().matches(options::OPT_mno_abicalls))
+      NeedNoAbiCalls = true;
----------------
I think this logic can be simplified to:

  bool UseAbiCalls = false;                                                                                            
  
  Arg *ABICallsArg =
      Args.getLastArg(options::OPT_mabicalls, options::OPT_mno_abicalls);                                              
  UseAbiCalls =
      !ABICallsArg || 
      (ABICallsArg && ABICallsArg->getOption().matches(options::OPT_mabicalls));                                       
  
  if (UseAbiCalls && IsN64 && NonPIC) {
    D.Diag(diag::warn_drv_unsupported_abicalls);                                                                       
    UseAbiCalls = false;                                                                                               
  }                                                                                                                    
  
  if (!UseAbiCalls)
    Features.push_back("+noabicalls");                                                                                 
  else
    Features.push_back("-noabicalls");         

As that way we immediately pick up the implicit case or explicit case of using -mabicalls and we can simply pick out the N64 && NonPic && UseAbiCalls case to warn and fix.

Although we don't need to add "-noabicalls" to the Features vector, the backend defaults to that, though the tests expect it.


Repository:
  rL LLVM

https://reviews.llvm.org/D36550