[llvm] r224901 - [X86][ISel] Fix a regression I introduced in r224884

[H.J. Lu]

On Mon, Dec 29, 2014 at 2:41 AM, Keno Fischer
<kfischer at college.harvard.edu> wrote:
> 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.
>
> On Sun, Dec 28, 2014 at 11:06 PM, Hal Finkel <hfinkel at anl.gov> wrote:
>>
>> ----- Original Message -----
>> > From: "Keno Fischer" <kfischer at college.harvard.edu>
>> > To: llvm-commits at cs.uiuc.edu
>> > Sent: Sunday, December 28, 2014 9:20:58 AM
>> > Subject: [llvm] r224901 - [X86][ISel] Fix a regression I introduced in
>> > r224884
>> >
>> > Author: kfischer
>> > Date: Sun Dec 28 09:20:57 2014
>> > New Revision: 224901
>> >
>> > URL: http://llvm.org/viewvc/llvm-project?rev=224901&view=rev
>> > Log:
>> > [X86][ISel] Fix a regression I introduced in r224884
>> >
>> > The else case ResultReg was not checked for validity.
>> > To my surprise, this case was not hit in any of the
>> > existing test cases. This includes a new test cases
>> > that tests this path.
>> >
>> > Also drop the `target triple` declaration from the
>> > original test as suggested by H.J. Lu, because
>> > apparently with it the test won't be run on Linux
>>
>> 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

Shouldn't it be "target", not "host"? llvm can be configured with

--target=xxx

>> 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.

That is true.  That is why I asked if the test was target specific
and my understanding is it was not specific to Darwin.

H.J.
>>
>>  -Hal
>>
>> >
>> > Added:
>> >     llvm/trunk/test/CodeGen/X86/large-code-model-isel.ll
>> > Modified:
>> >     llvm/trunk/lib/Target/X86/X86FastISel.cpp
>> >     llvm/trunk/test/CodeGen/X86/fast-isel-call-bool.ll
>> >
>> > Modified: llvm/trunk/lib/Target/X86/X86FastISel.cpp
>> > URL:
>> >
>> > http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86FastISel.cpp?rev=224901&r1=224900&r2=224901&view=diff
>> >
>> > ==============================================================================
>> > --- llvm/trunk/lib/Target/X86/X86FastISel.cpp (original)
>> > +++ llvm/trunk/lib/Target/X86/X86FastISel.cpp Sun Dec 28 09:20:57
>> > 2014
>> > @@ -2709,15 +2709,15 @@ bool X86FastISel::fastLowerCall(CallLowe
>> >
>> >        ResultReg =
>> >          fastEmit_ri(VT, VT, ISD::AND, ResultReg,
>> >          hasTrivialKill(PrevVal), 1);
>> > -
>> > -      if (!ResultReg)
>> > -        return false;
>> >      } else {
>> >        if (!isTypeLegal(Val->getType(), VT))
>> >          return false;
>> >        ResultReg = getRegForValue(Val);
>> >      }
>> >
>> > +    if (!ResultReg)
>> > +      return false;
>> > +
>> >      ArgRegs.push_back(ResultReg);
>> >      OutVTs.push_back(VT);
>> >    }
>> >
>> > Modified: llvm/trunk/test/CodeGen/X86/fast-isel-call-bool.ll
>> > URL:
>> >
>> > http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/fast-isel-call-bool.ll?rev=224901&r1=224900&r2=224901&view=diff
>> >
>> > ==============================================================================
>> > --- llvm/trunk/test/CodeGen/X86/fast-isel-call-bool.ll (original)
>> > +++ llvm/trunk/test/CodeGen/X86/fast-isel-call-bool.ll Sun Dec 28
>> > 09:20:57 2014
>> > @@ -1,8 +1,7 @@
>> > -; RUN: llc < %s -fast-isel -mcpu=core2 -O1 | FileCheck %s
>> > +; RUN: llc < %s -fast-isel -mcpu=core2 -march=x86-64 -O1 | FileCheck
>> > %s
>> >  ; See PR21557
>> >
>> >  target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
>> > -target triple = "x86_64-apple-darwin14.0.0"
>> >
>> >  declare i64 @bar(i1)
>> >
>> >
>> > Added: llvm/trunk/test/CodeGen/X86/large-code-model-isel.ll
>> > URL:
>> >
>> > http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/large-code-model-isel.ll?rev=224901&view=auto
>> >
>> > ==============================================================================
>> > --- llvm/trunk/test/CodeGen/X86/large-code-model-isel.ll (added)
>> > +++ llvm/trunk/test/CodeGen/X86/large-code-model-isel.ll Sun Dec 28
>> > 09:20:57 2014
>> > @@ -0,0 +1,13 @@
>> > +; RUN: llc < %s -code-model=large -mcpu=core2 -march=x86-64 -O0 |
>> > FileCheck %s
>> > +
>> > +target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
>> > +
>> > + at .str10 = external unnamed_addr constant [2 x i8], align 1
>> > +
>> > +define void @foo() {
>> > +; CHECK-LABEL: foo:
>> > +entry:
>> > +; CHECK: callq
>> > +  %call = call i64* undef(i64* undef, i8* getelementptr inbounds ([2
>> > x i8]* @.str10, i32 0, i32 0))
>> > +  ret void
>> > +}
>> >
>> >
>> > _______________________________________________
>> > llvm-commits mailing list
>> > llvm-commits at cs.uiuc.edu
>> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>> >
>>
>> --
>> Hal Finkel
>> Assistant Computational Scientist
>> Leadership Computing Facility
>> Argonne National Laboratory
>
>



-- 
H.J.