Hi Eli,

Thank you for the review.
I've moved relevant tests from test/CodeGen/xcore-abi.c to 
test/Driver/xcore-opts.c

I've added a predicate function to the handling of no_common:
...
if (KernelOrKext || isNoCommonDefault(getToolChain().getTriple())) {
    if (!Args.hasArg(options::OPT_fcommon))
      CmdArgs.push_back("-fno-common");
    Args.ClaimAllArgs(options::OPT_fno_common);
  }
...

static bool isNoCommonDefault(const llvm::Triple &Triple) {
  switch (Triple.getArch()) {
  default:
    return false;

  case llvm::Triple::xcore:
    return true;
  }
}

updated patch attached.

Robert

________________________________________
From: Eli Friedman [[email protected]]
Sent: 10 August 2013 05:42
To: Robert Lytton
Cc: [email protected]
Subject: Re: XCore target front end

On Wed, Aug 7, 2013 at 10:33 AM, Robert Lytton <[email protected]> wrote:
> Hi,
>
> I would like to commit the follow patch which adds support for the XCore
> target to clang.
> As I am relatively unfamiliar with clang I would not be surprised if there
> is additional work still need!
> I would be delighted if someone could verify I have not neglected anything
> or misunderstood the architecture in any way.

+void Clang::AddXCoreTargetArgs(const ArgList &Args,
+                                 ArgStringList &CmdArgs) const {
+  CmdArgs.push_back("-fno-common");
+}

This isn't right; if you want to default to -fno-common, you should
modify the existing OPT_fcommon handling.

+  // int getDwarfEHStackPointer(CodeGen::CodeGenModule &CGM) const {return 14;}

Why is this commented out?

You're missing tests for the driver changes.

Otherwise, looks fine.

-Eli

Attachment: PatchXCore
Description: PatchXCore

_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to