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/
+  "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 && 
  if (UseAbiCalls && IsN64 && NonPIC) {
    UseAbiCalls = false;                                                        
  if (!UseAbiCalls)

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.


cfe-commits mailing list

Reply via email to