From 283f5a198a0e3c9978ca426e64a3011b566c2581 Mon Sep 17 00:00:00 2001 From: Lucas Prates Date: Mon, 10 Jan 2022 10:19:27 +0000 Subject: [PATCH] [GlobalISel] Fix incorrect sign extension when combining G_INTTOPTR and G_PTR_ADD The GlobalISel combiner currently uses sign extension when manipulating the LHS constant when combining a sequence of the following sequence of machine instructions into a single constant: ``` %0:_(s32) = G_CONSTANT i32 %1:_(p0) = G_INTTOPTR %0:_(s32) %2:_(s64) = G_CONSTANT i64 %3:_(p0) = G_PTR_ADD %1:_, %2:_(s64) ``` This causes an issue when the bit width of the first contant and the target pointer size are different, as G_INTTOPTR has no sign extension semantics. This patch fixes this by capture an arbitrary precision in when matching the constant, allowing the matching function to correctly zero extend it. Reviewed By: arsenm Differential Revision: https://reviews.llvm.org/D116941 --- .../llvm/CodeGen/GlobalISel/CombinerHelper.h | 4 +-- .../llvm/CodeGen/GlobalISel/MIPatternMatch.h | 31 ++++++++++++++++++---- llvm/include/llvm/Target/GlobalISel/Combine.td | 2 +- llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp | 13 +++++---- .../AArch64/GlobalISel/combine-ptradd-int2ptr.mir | 15 +++++++++++ .../CodeGen/AArch64/GlobalISel/inttoptr_add.ll | 15 +++++++++++ 6 files changed, 67 insertions(+), 13 deletions(-) create mode 100644 llvm/test/CodeGen/AArch64/GlobalISel/inttoptr_add.ll diff --git a/llvm/include/llvm/CodeGen/GlobalISel/CombinerHelper.h b/llvm/include/llvm/CodeGen/GlobalISel/CombinerHelper.h index f3fa652..1d07d7d 100644 --- a/llvm/include/llvm/CodeGen/GlobalISel/CombinerHelper.h +++ b/llvm/include/llvm/CodeGen/GlobalISel/CombinerHelper.h @@ -353,8 +353,8 @@ public: std::pair &PtrRegAndCommute); // Transform G_PTR_ADD (G_PTRTOINT C1), C2 -> C1 + C2 - bool matchCombineConstPtrAddToI2P(MachineInstr &MI, int64_t &NewCst); - void applyCombineConstPtrAddToI2P(MachineInstr &MI, int64_t &NewCst); + bool matchCombineConstPtrAddToI2P(MachineInstr &MI, APInt &NewCst); + void applyCombineConstPtrAddToI2P(MachineInstr &MI, APInt &NewCst); /// Transform anyext(trunc(x)) to x. bool matchCombineAnyExtTrunc(MachineInstr &MI, Register &Reg); diff --git a/llvm/include/llvm/CodeGen/GlobalISel/MIPatternMatch.h b/llvm/include/llvm/CodeGen/GlobalISel/MIPatternMatch.h index 28bb8de..daf1ff0 100644 --- a/llvm/include/llvm/CodeGen/GlobalISel/MIPatternMatch.h +++ b/llvm/include/llvm/CodeGen/GlobalISel/MIPatternMatch.h @@ -13,6 +13,7 @@ #ifndef LLVM_CODEGEN_GLOBALISEL_MIPATTERNMATCH_H #define LLVM_CODEGEN_GLOBALISEL_MIPATTERNMATCH_H +#include "llvm/ADT/APInt.h" #include "llvm/CodeGen/GlobalISel/Utils.h" #include "llvm/CodeGen/MachineRegisterInfo.h" #include "llvm/IR/InstrTypes.h" @@ -59,11 +60,26 @@ inline OneNonDBGUse_match m_OneNonDBGUse(const SubPat &SP) { return SP; } -struct ConstantMatch { - int64_t &CR; - ConstantMatch(int64_t &C) : CR(C) {} +template +inline Optional matchConstant(Register, const MachineRegisterInfo &); + +template <> +inline Optional matchConstant(Register Reg, + const MachineRegisterInfo &MRI) { + return getIConstantVRegVal(Reg, MRI); +} + +template <> +inline Optional matchConstant(Register Reg, + const MachineRegisterInfo &MRI) { + return getIConstantVRegSExtVal(Reg, MRI); +} + +template struct ConstantMatch { + ConstT &CR; + ConstantMatch(ConstT &C) : CR(C) {} bool match(const MachineRegisterInfo &MRI, Register Reg) { - if (auto MaybeCst = getIConstantVRegSExtVal(Reg, MRI)) { + if (auto MaybeCst = matchConstant(Reg, MRI)) { CR = *MaybeCst; return true; } @@ -71,7 +87,12 @@ struct ConstantMatch { } }; -inline ConstantMatch m_ICst(int64_t &Cst) { return ConstantMatch(Cst); } +inline ConstantMatch m_ICst(APInt &Cst) { + return ConstantMatch(Cst); +} +inline ConstantMatch m_ICst(int64_t &Cst) { + return ConstantMatch(Cst); +} struct GCstAndRegMatch { Optional &ValReg; diff --git a/llvm/include/llvm/Target/GlobalISel/Combine.td b/llvm/include/llvm/Target/GlobalISel/Combine.td index 1d189c6..9736e52 100644 --- a/llvm/include/llvm/Target/GlobalISel/Combine.td +++ b/llvm/include/llvm/Target/GlobalISel/Combine.td @@ -391,7 +391,7 @@ def add_p2i_to_ptradd : GICombineRule< >; // Fold (ptr_add (int2ptr C1), C2) -> C1 + C2 -def const_ptradd_to_i2p_matchinfo : GIDefMatchData<"int64_t">; +def const_ptradd_to_i2p_matchinfo : GIDefMatchData<"APInt">; def const_ptradd_to_i2p: GICombineRule< (defs root:$root, const_ptradd_to_i2p_matchinfo:$info), (match (wip_match_opcode G_PTR_ADD):$root, diff --git a/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp b/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp index 9ba8cf0..ed1aa9d 100644 --- a/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp +++ b/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp @@ -2025,16 +2025,19 @@ void CombinerHelper::applyCombineAddP2IToPtrAdd( } bool CombinerHelper::matchCombineConstPtrAddToI2P(MachineInstr &MI, - int64_t &NewCst) { + APInt &NewCst) { auto &PtrAdd = cast(MI); Register LHS = PtrAdd.getBaseReg(); Register RHS = PtrAdd.getOffsetReg(); MachineRegisterInfo &MRI = Builder.getMF().getRegInfo(); - if (auto RHSCst = getIConstantVRegSExtVal(RHS, MRI)) { - int64_t Cst; + if (auto RHSCst = getIConstantVRegVal(RHS, MRI)) { + APInt Cst; if (mi_match(LHS, MRI, m_GIntToPtr(m_ICst(Cst)))) { - NewCst = Cst + *RHSCst; + auto DstTy = MRI.getType(PtrAdd.getReg(0)); + // G_INTTOPTR uses zero-extension + NewCst = Cst.zextOrTrunc(DstTy.getSizeInBits()); + NewCst += RHSCst->sextOrTrunc(DstTy.getSizeInBits()); return true; } } @@ -2043,7 +2046,7 @@ bool CombinerHelper::matchCombineConstPtrAddToI2P(MachineInstr &MI, } void CombinerHelper::applyCombineConstPtrAddToI2P(MachineInstr &MI, - int64_t &NewCst) { + APInt &NewCst) { auto &PtrAdd = cast(MI); Register Dst = PtrAdd.getReg(0); diff --git a/llvm/test/CodeGen/AArch64/GlobalISel/combine-ptradd-int2ptr.mir b/llvm/test/CodeGen/AArch64/GlobalISel/combine-ptradd-int2ptr.mir index c6ee994..40e5e8e 100644 --- a/llvm/test/CodeGen/AArch64/GlobalISel/combine-ptradd-int2ptr.mir +++ b/llvm/test/CodeGen/AArch64/GlobalISel/combine-ptradd-int2ptr.mir @@ -50,3 +50,18 @@ body: | %4:_(s64) = G_PTRTOINT %3 $x0 = COPY %4(s64) ... +--- +name: test_combine_zero_extend +body: | + bb.1: + liveins: $x0 + + ; CHECK-LABEL: name: test_combine_zero_extend + ; CHECK: [[C:%[0-9]+]]:_(p0) = G_CONSTANT i64 4291891236 + ; CHECK-NEXT: $x0 = COPY [[C]](p0) + %0:_(s32) = G_CONSTANT i32 -3076096 + %1:_(p0) = G_INTTOPTR %0:_(s32) + %2:_(s64) = G_CONSTANT i64 36 + %3:_(p0) = G_PTR_ADD %1:_, %2:_(s64) + $x0 = COPY %3 +... diff --git a/llvm/test/CodeGen/AArch64/GlobalISel/inttoptr_add.ll b/llvm/test/CodeGen/AArch64/GlobalISel/inttoptr_add.ll new file mode 100644 index 0000000..b7349b1 --- /dev/null +++ b/llvm/test/CodeGen/AArch64/GlobalISel/inttoptr_add.ll @@ -0,0 +1,15 @@ +; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py +; RUN: llc -mtriple=aarch64-none-eabi -global-isel -verify-machineinstrs %s -o - | FileCheck %s + +define dso_local void @fn() { +; CHECK-LABEL: fn: +; CHECK: // %bb.0: // %entry +; CHECK-NEXT: mov x8, #4132 +; CHECK-NEXT: mov w9, #1 +; CHECK-NEXT: movk x8, #65489, lsl #16 +; CHECK-NEXT: str w9, [x8] +; CHECK-NEXT: ret +entry: + store i32 1, i32* bitcast (i8* getelementptr inbounds (i8, i8* inttoptr (i32 -3076096 to i8*), i64 36) to i32*), align 4 + ret void +} -- 2.7.4