[llvm] r218632 - Add soft-float to the key for the subtarget lookup in the TargetMachine

David Blaikie dblaikie at gmail.com
Mon Sep 29 16:02:02 PDT 2014


On Mon, Sep 29, 2014 at 2:57 PM, Eric Christopher <echristo at gmail.com>
wrote:

> Author: echristo
> Date: Mon Sep 29 16:57:54 2014
> New Revision: 218632
>
> URL: http://llvm.org/viewvc/llvm-project?rev=218632&view=rev
> Log:
> Add soft-float to the key for the subtarget lookup in the TargetMachine
> map, this makes sure that we can compile the same code for two different
> ABIs (hard and soft float) in the same module.
>
> Update one testcase accordingly (and fix some confusing naming) and
> add a new testcase as well with the ordering swapped which would
> highlight the problem.
>
> Added:
>     llvm/trunk/test/CodeGen/Mips/mips16-hf-attr-2.ll
>       - copied, changed from r218631,
> llvm/trunk/test/CodeGen/Mips/mips16-hf-attr.ll
> Modified:
>     llvm/trunk/lib/Target/Mips/MipsTargetMachine.cpp
>     llvm/trunk/test/CodeGen/Mips/mips16-hf-attr.ll
>
> Modified: llvm/trunk/lib/Target/Mips/MipsTargetMachine.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/Mips/MipsTargetMachine.cpp?rev=218632&r1=218631&r2=218632&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/Target/Mips/MipsTargetMachine.cpp (original)
> +++ llvm/trunk/lib/Target/Mips/MipsTargetMachine.cpp Mon Sep 29 16:57:54
> 2014
> @@ -105,12 +105,24 @@ MipsTargetMachine::getSubtargetImpl(cons
>        !FnAttrs.getAttribute(AttributeSet::FunctionIndex, "nomips16")
>             .hasAttribute(Attribute::None);
>
> +  // FIXME: This is related to the code below to reset the target options,
> +  // we need to know whether or not the soft float flag is set on the
> +  // function before we can generate a subtarget. We also need to use
> +  // it as a key for the subtarget since that can be the only difference
> +  // between two functions.
> +  Attribute SFAttr =
> +      FnAttrs.getAttribute(AttributeSet::FunctionIndex, "use-soft-float");
> +  bool softFloat = !SFAttr.hasAttribute(Attribute::None)
>

This seems like a strange idiom. Is it the norm? (hmm, looked myself - most
code seems to do this as two lookups. hasAttr, then
getAttr+getValueAsString - which isn't really ideal... oh, and I see this
particular cliche of using "hasAttribute(Attribute::None)" is the norm in
this particular file... OK then :))


(should Attribute be boolean testable for "presence"? then this could be
written as:

  bool softFloat = Options.UseSoftFloat;
  if (Attribute SFAttr = FnAttr.getAttribute(AttributeSet::FunctionIndex,
"use-soft-float"))
    softFloat = SFAttr.getValueAsString() == "true";

perhaps?)


> +                       ? (SFAttr.getValueAsString() == "true" ? true :
> false)
>

This is a little redundant. "SFAttr.getValueAsString() == "true" is already
true/false, no need for the extra conditional operator, etc.


> +                       : Options.UseSoftFloat;
> +
>    if (hasMips16Attr)
>      FS += FS.empty() ? "+mips16" : ",+mips16";
>    else if (hasNoMips16Attr)
>      FS += FS.empty() ? "-mips16" : ",-mips16";
>
> -  auto &I = SubtargetMap[CPU + FS];
> +  auto &I = SubtargetMap[CPU + FS + (softFloat ? "use-soft-float=true"
> +                                               : "use-soft-float=false")];
>

Hadn't gotten around to looking at the original patch that introduced the
SubtargetMap - but I assume you've plans to replace this with something
better at some point? (to avoid building new strings each time and
canonicalize the features, etc (if there's any canonicalization required? I
don't know))


>    if (!I) {
>      // This needs to be done before we create a new subtarget since any
>      // creation will depend on the TM and the code generation flags on the
>
> Copied: llvm/trunk/test/CodeGen/Mips/mips16-hf-attr-2.ll (from r218631,
> llvm/trunk/test/CodeGen/Mips/mips16-hf-attr.ll)
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/Mips/mips16-hf-attr-2.ll?p2=llvm/trunk/test/CodeGen/Mips/mips16-hf-attr-2.ll&p1=llvm/trunk/test/CodeGen/Mips/mips16-hf-attr.ll&r1=218631&r2=218632&rev=218632&view=diff
>
> ==============================================================================
> --- llvm/trunk/test/CodeGen/Mips/mips16-hf-attr.ll (original)
> +++ llvm/trunk/test/CodeGen/Mips/mips16-hf-attr-2.ll Mon Sep 29 16:57:54
> 2014
> @@ -3,16 +3,16 @@
>  ; RUN: llc -mtriple=mipsel-linux-gnu -march=mipsel \
>  ; RUN:     -mcpu=mips16 -relocation-model=pic < %s | FileCheck %s
>
> -define void @bar_sf() #0 {
> +define void @bar_sf() #1 {
>  ; CHECK: bar_sf:
>  entry:
>    %call1 = call float @foo(float 1.000000e+00)
> -; CHECK: lw $2, %call16(foo)($3)
> -; CHECK: lw $5, %got(__mips16_call_stub_sf_1)($3)
> +; CHECK: lw $3, %call16(foo)($2)
> +; CHECK-NOT: lw $5, %got(__mips16_call_stub_sf_1)($3)
>    ret void
>  }
>
> -define void @bar_hf() #1 {
> +define void @bar_hf() #0 {
>  ; CHECK: bar_hf:
>  entry:
>    %call1 = call float @foo(float 1.000000e+00)
>
> Modified: llvm/trunk/test/CodeGen/Mips/mips16-hf-attr.ll
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/Mips/mips16-hf-attr.ll?rev=218632&r1=218631&r2=218632&view=diff
>
> ==============================================================================
> --- llvm/trunk/test/CodeGen/Mips/mips16-hf-attr.ll (original)
> +++ llvm/trunk/test/CodeGen/Mips/mips16-hf-attr.ll Mon Sep 29 16:57:54 2014
> @@ -3,8 +3,8 @@
>  ; RUN: llc -mtriple=mipsel-linux-gnu -march=mipsel \
>  ; RUN:     -mcpu=mips16 -relocation-model=pic < %s | FileCheck %s
>
> -define void @bar_sf() #0 {
> -; CHECK: bar_sf:
> +define void @bar_hf() #0 {
> +; CHECK: bar_hf:
>  entry:
>    %call1 = call float @foo(float 1.000000e+00)
>  ; CHECK: lw $2, %call16(foo)($3)
> @@ -12,12 +12,12 @@ entry:
>    ret void
>  }
>
> -define void @bar_hf() #1 {
> -; CHECK: bar_hf:
> +define void @bar_sf() #1 {
> +; CHECK: bar_sf:
>  entry:
>    %call1 = call float @foo(float 1.000000e+00)
> -; CHECK: lw $2, %call16(foo)($3)
> -; CHECK: lw $5, %got(__mips16_call_stub_sf_1)($3)
> +; CHECK: lw $3, %call16(foo)($2)
> +; CHECK-NOT: lw $5, %got(__mips16_call_stub_sf_1)($3)
>    ret void
>  }
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140929/e61ac75d/attachment.html>


More information about the llvm-commits mailing list