<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Sep 29, 2014 at 2:57 PM, Eric Christopher <span dir="ltr"><<a href="mailto:echristo@gmail.com" target="_blank">echristo@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: echristo<br>
Date: Mon Sep 29 16:57:54 2014<br>
New Revision: 218632<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=218632&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=218632&view=rev</a><br>
Log:<br>
Add soft-float to the key for the subtarget lookup in the TargetMachine<br>
map, this makes sure that we can compile the same code for two different<br>
ABIs (hard and soft float) in the same module.<br>
<br>
Update one testcase accordingly (and fix some confusing naming) and<br>
add a new testcase as well with the ordering swapped which would<br>
highlight the problem.<br>
<br>
Added:<br>
llvm/trunk/test/CodeGen/Mips/mips16-hf-attr-2.ll<br>
- copied, changed from r218631, llvm/trunk/test/CodeGen/Mips/mips16-hf-attr.ll<br>
Modified:<br>
llvm/trunk/lib/Target/Mips/MipsTargetMachine.cpp<br>
llvm/trunk/test/CodeGen/Mips/mips16-hf-attr.ll<br>
<br>
Modified: llvm/trunk/lib/Target/Mips/MipsTargetMachine.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/Mips/MipsTargetMachine.cpp?rev=218632&r1=218631&r2=218632&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/Mips/MipsTargetMachine.cpp?rev=218632&r1=218631&r2=218632&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Target/Mips/MipsTargetMachine.cpp (original)<br>
+++ llvm/trunk/lib/Target/Mips/MipsTargetMachine.cpp Mon Sep 29 16:57:54 2014<br>
@@ -105,12 +105,24 @@ MipsTargetMachine::getSubtargetImpl(cons<br>
!FnAttrs.getAttribute(AttributeSet::FunctionIndex, "nomips16")<br>
.hasAttribute(Attribute::None);<br>
<br>
+ // FIXME: This is related to the code below to reset the target options,<br>
+ // we need to know whether or not the soft float flag is set on the<br>
+ // function before we can generate a subtarget. We also need to use<br>
+ // it as a key for the subtarget since that can be the only difference<br>
+ // between two functions.<br>
+ Attribute SFAttr =<br>
+ FnAttrs.getAttribute(AttributeSet::FunctionIndex, "use-soft-float");<br>
+ bool softFloat = !SFAttr.hasAttribute(Attribute::None)<br></blockquote><div><br></div><div>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 :))<br><br><br>(should Attribute be boolean testable for "presence"? then this could be written as:<br><br> bool softFloat = Options.UseSoftFloat;<br> if (Attribute SFAttr = FnAttr.getAttribute(AttributeSet::FunctionIndex, "use-soft-float"))<br> softFloat = SFAttr.getValueAsString() == "true";</div><div><br>perhaps?)</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ ? (SFAttr.getValueAsString() == "true" ? true : false)<br></blockquote><div><br></div><div>This is a little redundant. "SFAttr.getValueAsString() == "true" is already true/false, no need for the extra conditional operator, etc.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ : Options.UseSoftFloat;<br>
+<br>
if (hasMips16Attr)<br>
FS += FS.empty() ? "+mips16" : ",+mips16";<br>
else if (hasNoMips16Attr)<br>
FS += FS.empty() ? "-mips16" : ",-mips16";<br>
<br>
- auto &I = SubtargetMap[CPU + FS];<br>
+ auto &I = SubtargetMap[CPU + FS + (softFloat ? "use-soft-float=true"<br>
+ : "use-soft-float=false")];<br></blockquote><div><br>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))<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
if (!I) {<br>
// This needs to be done before we create a new subtarget since any<br>
// creation will depend on the TM and the code generation flags on the<br>
<br>
Copied: llvm/trunk/test/CodeGen/Mips/mips16-hf-attr-2.ll (from r218631, llvm/trunk/test/CodeGen/Mips/mips16-hf-attr.ll)<br>
URL: <a href="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" target="_blank">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</a><br>
==============================================================================<br>
--- llvm/trunk/test/CodeGen/Mips/mips16-hf-attr.ll (original)<br>
+++ llvm/trunk/test/CodeGen/Mips/mips16-hf-attr-2.ll Mon Sep 29 16:57:54 2014<br>
@@ -3,16 +3,16 @@<br>
; RUN: llc -mtriple=mipsel-linux-gnu -march=mipsel \<br>
; RUN: -mcpu=mips16 -relocation-model=pic < %s | FileCheck %s<br>
<br>
-define void @bar_sf() #0 {<br>
+define void @bar_sf() #1 {<br>
; CHECK: bar_sf:<br>
entry:<br>
%call1 = call float @foo(float 1.000000e+00)<br>
-; CHECK: lw $2, %call16(foo)($3)<br>
-; CHECK: lw $5, %got(__mips16_call_stub_sf_1)($3)<br>
+; CHECK: lw $3, %call16(foo)($2)<br>
+; CHECK-NOT: lw $5, %got(__mips16_call_stub_sf_1)($3)<br>
ret void<br>
}<br>
<br>
-define void @bar_hf() #1 {<br>
+define void @bar_hf() #0 {<br>
; CHECK: bar_hf:<br>
entry:<br>
%call1 = call float @foo(float 1.000000e+00)<br>
<br>
Modified: llvm/trunk/test/CodeGen/Mips/mips16-hf-attr.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/Mips/mips16-hf-attr.ll?rev=218632&r1=218631&r2=218632&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/Mips/mips16-hf-attr.ll?rev=218632&r1=218631&r2=218632&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/test/CodeGen/Mips/mips16-hf-attr.ll (original)<br>
+++ llvm/trunk/test/CodeGen/Mips/mips16-hf-attr.ll Mon Sep 29 16:57:54 2014<br>
@@ -3,8 +3,8 @@<br>
; RUN: llc -mtriple=mipsel-linux-gnu -march=mipsel \<br>
; RUN: -mcpu=mips16 -relocation-model=pic < %s | FileCheck %s<br>
<br>
-define void @bar_sf() #0 {<br>
-; CHECK: bar_sf:<br>
+define void @bar_hf() #0 {<br>
+; CHECK: bar_hf:<br>
entry:<br>
%call1 = call float @foo(float 1.000000e+00)<br>
; CHECK: lw $2, %call16(foo)($3)<br>
@@ -12,12 +12,12 @@ entry:<br>
ret void<br>
}<br>
<br>
-define void @bar_hf() #1 {<br>
-; CHECK: bar_hf:<br>
+define void @bar_sf() #1 {<br>
+; CHECK: bar_sf:<br>
entry:<br>
%call1 = call float @foo(float 1.000000e+00)<br>
-; CHECK: lw $2, %call16(foo)($3)<br>
-; CHECK: lw $5, %got(__mips16_call_stub_sf_1)($3)<br>
+; CHECK: lw $3, %call16(foo)($2)<br>
+; CHECK-NOT: lw $5, %got(__mips16_call_stub_sf_1)($3)<br>
ret void<br>
}<br>
<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div></div>