<div dir="ltr">Ok, what's the general rule on whether to include `target triple` and `target datalayout` in the tests? Also, H.J. could you comment on why you recommend omitting it. </div><div class="gmail_extra"><br><div class="gmail_quote">On Sun, Dec 28, 2014 at 11:06 PM, Hal Finkel <span dir="ltr"><<a href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">----- Original Message -----<br>
> From: "Keno Fischer" <<a href="mailto:kfischer@college.harvard.edu">kfischer@college.harvard.edu</a>><br>
> To: <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
> Sent: Sunday, December 28, 2014 9:20:58 AM<br>
> Subject: [llvm] r224901 - [X86][ISel] Fix a regression I introduced in r224884<br>
><br>
> Author: kfischer<br>
> Date: Sun Dec 28 09:20:57 2014<br>
> New Revision: 224901<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=224901&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=224901&view=rev</a><br>
> Log:<br>
> [X86][ISel] Fix a regression I introduced in r224884<br>
><br>
> The else case ResultReg was not checked for validity.<br>
> To my surprise, this case was not hit in any of the<br>
> existing test cases. This includes a new test cases<br>
> that tests this path.<br>
><br>
> Also drop the `target triple` declaration from the<br>
> original test as suggested by H.J. Lu, because<br>
> apparently with it the test won't be run on Linux<br>
<br>
</span>No, that's not right. All of the llc tests are run, regardless of the host; adding a triple is generally done for better test stability (so that aspects of the host's configuration don't leak into the code generation for the test, potentially making the test behave differently on different hosts. If there is any chance that the code generation could be sensitive to features of the host's triple in some way relevant to what the test is checking, you should definitely have a triple. Otherwise, omitting it is fine.<br>
<br>
-Hal<br>
<div class="HOEnZb"><div class="h5"><br>
><br>
> Added:<br>
> llvm/trunk/test/CodeGen/X86/large-code-model-isel.ll<br>
> Modified:<br>
> llvm/trunk/lib/Target/X86/X86FastISel.cpp<br>
> llvm/trunk/test/CodeGen/X86/fast-isel-call-bool.ll<br>
><br>
> Modified: llvm/trunk/lib/Target/X86/X86FastISel.cpp<br>
> URL:<br>
> <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86FastISel.cpp?rev=224901&r1=224900&r2=224901&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86FastISel.cpp?rev=224901&r1=224900&r2=224901&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/lib/Target/X86/X86FastISel.cpp (original)<br>
> +++ llvm/trunk/lib/Target/X86/X86FastISel.cpp Sun Dec 28 09:20:57<br>
> 2014<br>
> @@ -2709,15 +2709,15 @@ bool X86FastISel::fastLowerCall(CallLowe<br>
><br>
> ResultReg =<br>
> fastEmit_ri(VT, VT, ISD::AND, ResultReg,<br>
> hasTrivialKill(PrevVal), 1);<br>
> -<br>
> - if (!ResultReg)<br>
> - return false;<br>
> } else {<br>
> if (!isTypeLegal(Val->getType(), VT))<br>
> return false;<br>
> ResultReg = getRegForValue(Val);<br>
> }<br>
><br>
> + if (!ResultReg)<br>
> + return false;<br>
> +<br>
> ArgRegs.push_back(ResultReg);<br>
> OutVTs.push_back(VT);<br>
> }<br>
><br>
> Modified: llvm/trunk/test/CodeGen/X86/fast-isel-call-bool.ll<br>
> URL:<br>
> <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/fast-isel-call-bool.ll?rev=224901&r1=224900&r2=224901&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/fast-isel-call-bool.ll?rev=224901&r1=224900&r2=224901&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/test/CodeGen/X86/fast-isel-call-bool.ll (original)<br>
> +++ llvm/trunk/test/CodeGen/X86/fast-isel-call-bool.ll Sun Dec 28<br>
> 09:20:57 2014<br>
> @@ -1,8 +1,7 @@<br>
> -; RUN: llc < %s -fast-isel -mcpu=core2 -O1 | FileCheck %s<br>
> +; RUN: llc < %s -fast-isel -mcpu=core2 -march=x86-64 -O1 | FileCheck<br>
> %s<br>
> ; See PR21557<br>
><br>
> target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"<br>
> -target triple = "x86_64-apple-darwin14.0.0"<br>
><br>
> declare i64 @bar(i1)<br>
><br>
><br>
> Added: llvm/trunk/test/CodeGen/X86/large-code-model-isel.ll<br>
> URL:<br>
> <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/large-code-model-isel.ll?rev=224901&view=auto" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/large-code-model-isel.ll?rev=224901&view=auto</a><br>
> ==============================================================================<br>
> --- llvm/trunk/test/CodeGen/X86/large-code-model-isel.ll (added)<br>
> +++ llvm/trunk/test/CodeGen/X86/large-code-model-isel.ll Sun Dec 28<br>
> 09:20:57 2014<br>
> @@ -0,0 +1,13 @@<br>
> +; RUN: llc < %s -code-model=large -mcpu=core2 -march=x86-64 -O0 |<br>
> FileCheck %s<br>
> +<br>
> +target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"<br>
> +<br>
> +@.str10 = external unnamed_addr constant [2 x i8], align 1<br>
> +<br>
> +define void @foo() {<br>
> +; CHECK-LABEL: foo:<br>
> +entry:<br>
> +; CHECK: callq<br>
> + %call = call i64* undef(i64* undef, i8* getelementptr inbounds ([2<br>
> x i8]* @.str10, i32 0, i32 0))<br>
> + ret void<br>
> +}<br>
><br>
><br>
> _______________________________________________<br>
> llvm-commits mailing list<br>
> <a href="mailto:llvm-commits@cs.uiuc.edu">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>
><br>
<br>
--<br>
</div></div><span class="HOEnZb"><font color="#888888">Hal Finkel<br>
Assistant Computational Scientist<br>
Leadership Computing Facility<br>
Argonne National Laboratory<br>
</font></span></blockquote></div><br></div>