This is an archive of the discontinued LLVM Phabricator instance.

[mips] Separated mips specific -Wa options, so that they are not checked on other platforms.
ClosedPublic

Authored by s.egerton on Sep 23 2015, 7:55 AM.

Diff Detail

Event Timeline

s.egerton updated this revision to Diff 35502.Sep 23 2015, 7:55 AM
s.egerton retitled this revision from to [mips] Separated mips specific -Wa options, so that they are not checked on other platforms..
s.egerton updated this object.
s.egerton added reviewers: dsanders, vkalintiris.
s.egerton added a subscriber: cfe-commits.
dsanders edited edge metadata.Sep 24 2015, 3:22 AM
dsanders added subscribers: rengolin, joerg.

+Renato and Joerg

I was going to say I think it's ok and the optimizer should be smart enough to factor out the common IsMips check but I've just realized there may be a better way. The current code is using an else after an (implicit) continue. If we made that continue explicit, we could make this code a bit neater and have a place to add target specific options.

I'm thinking something like:

for (...) {
  ...

  auto Arch = C.getDefaultToolChain().getArch();

  if (C.getDefaultToolChain().getArch() == llvm::Triple::mips ||
      C.getDefaultToolChain().getArch() == llvm::Triple::mipsel ||
      C.getDefaultToolChain().getArch() == llvm::Triple::mips64 ||
      C.getDefaultToolChain().getArch() == llvm::Triple::mips64el)
    if (mips::CollectArgsForIntegratedAssembler(...)
      continue;

  if (Value == "-force_cpusubtype_ALL")
    continue;

  ...

  D.Diag(diag::err_drv_unsupported_option_argument)
      << A->getOption().getName() << Value;
}

Thoughts?

s.egerton updated this revision to Diff 36252.Oct 1 2015, 8:47 AM
s.egerton edited edge metadata.

Responded to comments made on the mailing list.

dsanders accepted this revision.Oct 6 2015, 2:04 AM
dsanders edited edge metadata.

LGTM

This revision is now accepted and ready to land.Oct 6 2015, 2:04 AM
dsanders closed this revision.Oct 27 2015, 11:06 AM