Skip site navigation (1)Skip section navigation (2) 
Date:      Sat, 7 Feb 2015 12:50:34 +0000 (UTC)
From:      Dimitry Andric <dim@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r278349 - head/contrib/llvm/lib/Target/X86
Message-ID:  <201502071250.t17CoY49076516@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: dim
Date: Sat Feb  7 12:50:33 2015
New Revision: 278349
URL: https://svnweb.freebsd.org/changeset/base/278349

Log:
  Pull in r224884 from upstream llvm trunk (by Keno Fischer):
  
    [FastIsel][X86] Fix invalid register replacement for bool args
  
    Summary:
    Consider the following IR:
  
     %3 = load i8* undef
     %4 = trunc i8 %3 to i1
     %5 = call %jl_value_t.0* @foo(..., i1 %4, ...)
     ret %jl_value_t.0* %5
  
    Bools (that are the result of direct truncs) are lowered as whatever
    the argument to the trunc was and a "and 1", causing the part of the
    MBB responsible for this argument to look something like this:
  
     %vreg8<def,tied1> = AND8ri %vreg7<kill,tied0>, 1, %EFLAGS<imp-def>; GR8:%vreg8,%vreg7
  
    Later, when the load is lowered, it will insert
  
     %vreg15<def> = MOV8rm %vreg14, 1, %noreg, 0, %noreg; mem:LD1[undef] GR8:%vreg15 GR64:%vreg14
  
    but remember to (at the end of isel) replace vreg7 by vreg15. Now for
    the bug. In fast isel lowering, we mistakenly mark vreg8 as the result
    of the load instead of the trunc. This adds a fixup to have
    vreg8 replaced by whatever the result of the load is as well, so
    we end up with
  
     %vreg15<def,tied1> = AND8ri %vreg15<kill,tied0>, 1, %EFLAGS<imp-def>; GR8:%vreg15
  
    which is an SSA violation and causes problems later down the road.
  
    This fixes PR21557.
  
    Test Plan: Test test case from PR21557 is added to the test suite.
  
    Reviewers: ributzka
  
    Reviewed By: ributzka
  
    Subscribers: llvm-commits
  
    Differential Revision: http://reviews.llvm.org/D6245
  
  This fixes a possible assertion failure when compiling toolbox.cxx from
  LibreOffice 4.3.5.
  
  Reported by:	kwm

Modified:
  head/contrib/llvm/lib/Target/X86/X86FastISel.cpp

Modified: head/contrib/llvm/lib/Target/X86/X86FastISel.cpp
==============================================================================
--- head/contrib/llvm/lib/Target/X86/X86FastISel.cpp	Sat Feb  7 12:20:33 2015	(r278348)
+++ head/contrib/llvm/lib/Target/X86/X86FastISel.cpp	Sat Feb  7 12:50:33 2015	(r278349)
@@ -2699,6 +2699,9 @@ bool X86FastISel::FastLowerCall(CallLowe
                        TM.Options.GuaranteedTailCallOpt))
     return false;
 
+  SmallVector<MVT, 16> OutVTs;
+  SmallVector<unsigned, 16> ArgRegs;
+
   // If this is a constant i1/i8/i16 argument, promote to i32 to avoid an extra
   // instruction. This is safe because it is common to all FastISel supported
   // calling conventions on x86.
@@ -2716,28 +2719,34 @@ bool X86FastISel::FastLowerCall(CallLowe
 
     // Passing bools around ends up doing a trunc to i1 and passing it.
     // Codegen this as an argument + "and 1".
-    if (auto *TI = dyn_cast<TruncInst>(Val)) {
-      if (TI->getType()->isIntegerTy(1) && CLI.CS &&
-          (TI->getParent() == CLI.CS->getInstruction()->getParent()) &&
-          TI->hasOneUse()) {
-        Val = cast<TruncInst>(Val)->getOperand(0);
-        unsigned ResultReg = getRegForValue(Val);
+    MVT VT;
+    auto *TI = dyn_cast<TruncInst>(Val);
+    unsigned ResultReg;
+    if (TI && TI->getType()->isIntegerTy(1) && CLI.CS &&
+              (TI->getParent() == CLI.CS->getInstruction()->getParent()) &&
+              TI->hasOneUse()) {
+      Value *PrevVal = TI->getOperand(0);
+      ResultReg = getRegForValue(PrevVal);
+
+      if (!ResultReg)
+        return false;
 
-        if (!ResultReg)
-          return false;
+      if (!isTypeLegal(PrevVal->getType(), VT))
+        return false;
 
-        MVT ArgVT;
-        if (!isTypeLegal(Val->getType(), ArgVT))
-          return false;
+      ResultReg =
+        FastEmit_ri(VT, VT, ISD::AND, ResultReg, hasTrivialKill(PrevVal), 1);
 
-        ResultReg =
-          FastEmit_ri(ArgVT, ArgVT, ISD::AND, ResultReg, Val->hasOneUse(), 1);
-
-        if (!ResultReg)
-          return false;
-        UpdateValueMap(Val, ResultReg);
-      }
+      if (!ResultReg)
+        return false;
+    } else {
+      if (!isTypeLegal(Val->getType(), VT))
+        return false;
+      ResultReg = getRegForValue(Val);
     }
+
+    ArgRegs.push_back(ResultReg);
+    OutVTs.push_back(VT);
   }
 
   // Analyze operands of the call, assigning locations to each operand.
@@ -2749,13 +2758,6 @@ bool X86FastISel::FastLowerCall(CallLowe
   if (IsWin64)
     CCInfo.AllocateStack(32, 8);
 
-  SmallVector<MVT, 16> OutVTs;
-  for (auto *Val : OutVals) {
-    MVT VT;
-    if (!isTypeLegal(Val->getType(), VT))
-      return false;
-    OutVTs.push_back(VT);
-  }
   CCInfo.AnalyzeCallOperands(OutVTs, OutFlags, CC_X86);
 
   // Get a count of how many bytes are to be pushed on the stack.
@@ -2777,9 +2779,7 @@ bool X86FastISel::FastLowerCall(CallLowe
     if (ArgVT == MVT::x86mmx)
       return false;
 
-    unsigned ArgReg = getRegForValue(ArgVal);
-    if (!ArgReg)
-      return false;
+    unsigned ArgReg = ArgRegs[VA.getValNo()];
 
     // Promote the value if needed.
     switch (VA.getLocInfo()) {

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201502071250.t17CoY49076516>