From 880580b88f9deed575104168e85f242401f8a92e Mon Sep 17 00:00:00 2001 From: Robin Morisset Date: Tue, 7 Oct 2014 23:53:57 +0000 Subject: [PATCH] [X86] Fix a bug with fetch_add(INT32_MIN) Summary: Fix pr21099 The pseudocode of what we were doing (spread through two functions) was: if (operand.doesNotFitIn32Bits()) Opc.initializeWithFoo(); if (operand < 0) operand = -operand; if (operand.doesFitIn8Bits()) Opc.initializeWithBar(); else if (operand.doesFitIn32Bits()) Opc.initializeWithBlah(); doStuff(Opc); So for operand == INT32_MIN, Opc was never initialized because the operand changes from fitting in 32 bits to not fitting, causing the various bugs/error messages noted by pr21099. This patch adds an extra test at the beginning for this case, and an llvm_unreachable to have better error message if the operand ends up not fitting in 32-bits at the end. Test Plan: new test + make check Reviewers: jfb Subscribers: llvm-commits Differential Revision: http://reviews.llvm.org/D5655 llvm-svn: 219257 --- llvm/lib/Target/X86/X86ISelDAGToDAG.cpp | 9 +++++++++ llvm/test/CodeGen/X86/pr21099.ll | 10 ++++++++++ 2 files changed, 19 insertions(+) create mode 100644 llvm/test/CodeGen/X86/pr21099.ll diff --git a/llvm/lib/Target/X86/X86ISelDAGToDAG.cpp b/llvm/lib/Target/X86/X86ISelDAGToDAG.cpp index 429c8bdbdf0a..f72c70c58c10 100644 --- a/llvm/lib/Target/X86/X86ISelDAGToDAG.cpp +++ b/llvm/lib/Target/X86/X86ISelDAGToDAG.cpp @@ -34,6 +34,7 @@ #include "llvm/Support/raw_ostream.h" #include "llvm/Target/TargetMachine.h" #include "llvm/Target/TargetOptions.h" +#include using namespace llvm; #define DEBUG_TYPE "x86-isel" @@ -1712,6 +1713,12 @@ static SDValue getAtomicLoadArithTargetConstant(SelectionDAG *CurDAG, // Quit if not 32-bit imm. if ((int32_t)CNVal != CNVal) return Val; + // Quit if INT32_MIN: it would be negated as it is negative and overflow, + // producing an immediate that does not fit in the 32 bits available for + // an immediate operand to sub. However, it still fits in 32 bits for the + // add (since it is not negated) so we can return target-constant. + if (CNVal == INT32_MIN) + return CurDAG->getTargetConstant(CNVal, NVT); // For atomic-load-add, we could do some optimizations. if (Op == ADD) { // Translate to INC/DEC if ADD by 1 or -1. @@ -1824,6 +1831,8 @@ SDNode *X86DAGToDAGISel::SelectAtomicLoadArith(SDNode *Node, MVT NVT) { Opc = AtomicOpcTbl[Op][SextConstantI64]; else if (i64immSExt32(Val.getNode())) Opc = AtomicOpcTbl[Op][ConstantI64]; + else + llvm_unreachable("True 64 bits constant in SelectAtomicLoadArith"); } else Opc = AtomicOpcTbl[Op][I64]; break; diff --git a/llvm/test/CodeGen/X86/pr21099.ll b/llvm/test/CodeGen/X86/pr21099.ll new file mode 100644 index 000000000000..07292c125eea --- /dev/null +++ b/llvm/test/CodeGen/X86/pr21099.ll @@ -0,0 +1,10 @@ +; RUN: llc < %s -O2 -march=x86-64 -verify-machineinstrs | FileCheck %s + +define void @pr21099(i64* %p) { +; CHECK-LABEL: pr21099 +; CHECK: lock +; CHECK-NEXT: addq $-2147483648 +; This number is INT32_MIN: 0x80000000UL + %1 = atomicrmw add i64* %p, i64 -2147483648 seq_cst + ret void +}