From c6a82d971b6ef9d1d5f8745f023673a8009b41a1 Mon Sep 17 00:00:00 2001 From: Krzysztof Parzyszek Date: Thu, 29 Mar 2018 13:54:31 +0000 Subject: [PATCH] [Hexagon] Aid bit-reverse load intrinsics lowering with bitcode The conversion of operatios to bitcode helps to eliminate an additional store in certain cases. We used to lower these load intrinsics in DAG to DAG conversion by which time, the "Dead Store Elimination" pass is already run. There is an associated LLVM patch. Patch by Sumanth Gundapaneni. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@328776 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/Basic/BuiltinsHexagon.def | 22 +++++----- lib/CodeGen/CGBuiltin.cpp | 51 ++++++++++++++++++++++ test/CodeGen/builtins-hexagon.c | 26 ++++++------ test/CodeGen/hexagon-brev-ld-ptr-incdec.c | 52 +++++++++++++++++++++++ test/CodeGen/hexagon-brev-store-elm.c | 46 ++++++++++++++++++++ 5 files changed, 174 insertions(+), 23 deletions(-) create mode 100644 test/CodeGen/hexagon-brev-ld-ptr-incdec.c create mode 100644 test/CodeGen/hexagon-brev-store-elm.c diff --git a/include/clang/Basic/BuiltinsHexagon.def b/include/clang/Basic/BuiltinsHexagon.def index c92a236c5c..f976720d11 100644 --- a/include/clang/Basic/BuiltinsHexagon.def +++ b/include/clang/Basic/BuiltinsHexagon.def @@ -17,23 +17,23 @@ // The builtins below are not autogenerated from iset.py. // Make sure you do not overwrite these. -BUILTIN(__builtin_brev_ldd, "LLi*LLi*LLi*i", "") -BUILTIN(__builtin_brev_ldw, "i*i*i*i", "") -BUILTIN(__builtin_brev_ldh, "s*s*s*i", "") -BUILTIN(__builtin_brev_lduh, "Us*Us*Us*i", "") -BUILTIN(__builtin_brev_ldb, "c*c*c*i", "") -BUILTIN(__builtin_brev_ldub, "Uc*Uc*Uc*i", "") +BUILTIN(__builtin_brev_ldd, "v*LLi*CLLi*iC", "") +BUILTIN(__builtin_brev_ldw, "v*i*Ci*iC", "") +BUILTIN(__builtin_brev_ldh, "v*s*Cs*iC", "") +BUILTIN(__builtin_brev_lduh, "v*Us*CUs*iC", "") +BUILTIN(__builtin_brev_ldb, "v*Sc*CSc*iC", "") +BUILTIN(__builtin_brev_ldub, "v*Uc*CUc*iC", "") BUILTIN(__builtin_circ_ldd, "LLi*LLi*LLi*iIi", "") BUILTIN(__builtin_circ_ldw, "i*i*i*iIi", "") BUILTIN(__builtin_circ_ldh, "s*s*s*iIi", "") BUILTIN(__builtin_circ_lduh, "Us*Us*Us*iIi", "") BUILTIN(__builtin_circ_ldb, "c*c*c*iIi", "") BUILTIN(__builtin_circ_ldub, "Uc*Uc*Uc*iIi", "") -BUILTIN(__builtin_brev_std, "LLi*LLi*LLii", "") -BUILTIN(__builtin_brev_stw, "i*i*ii", "") -BUILTIN(__builtin_brev_sth, "s*s*ii", "") -BUILTIN(__builtin_brev_sthhi, "s*s*ii", "") -BUILTIN(__builtin_brev_stb, "c*c*ii", "") +BUILTIN(__builtin_brev_std, "LLi*CLLi*LLiiC", "") +BUILTIN(__builtin_brev_stw, "i*Ci*iiC", "") +BUILTIN(__builtin_brev_sth, "s*Cs*iiC", "") +BUILTIN(__builtin_brev_sthhi, "s*Cs*iiC", "") +BUILTIN(__builtin_brev_stb, "c*Cc*iiC", "") BUILTIN(__builtin_circ_std, "LLi*LLi*LLiiIi", "") BUILTIN(__builtin_circ_stw, "i*i*iiIi", "") BUILTIN(__builtin_circ_sth, "s*s*iiIi", "") diff --git a/lib/CodeGen/CGBuiltin.cpp b/lib/CodeGen/CGBuiltin.cpp index 8458d4d8a2..79e571c2a6 100644 --- a/lib/CodeGen/CGBuiltin.cpp +++ b/lib/CodeGen/CGBuiltin.cpp @@ -10820,6 +10820,45 @@ Value *CodeGenFunction::EmitHexagonBuiltinExpr(unsigned BuiltinID, return Builder.CreateAlignedStore(NewBase, LV, Dest.getAlignment()); }; + // Handle the conversion of bit-reverse load intrinsics to bit code. + // The intrinsic call after this function only reads from memory and the + // write to memory is dealt by the store instruction. + auto MakeBrevLd = [&](unsigned IntID, llvm::Type *DestTy) { + // The intrinsic generates one result, which is the new value for the base + // pointer. It needs to be returned. The result of the load instruction is + // passed to intrinsic by address, so the value needs to be stored. + llvm::Value *BaseAddress = + Builder.CreateBitCast(EmitScalarExpr(E->getArg(0)), Int8PtrTy); + + // Expressions like &(*pt++) will be incremented per evaluation. + // EmitPointerWithAlignment and EmitScalarExpr evaluates the expression + // per call. + Address DestAddr = EmitPointerWithAlignment(E->getArg(1)); + DestAddr = Address(Builder.CreateBitCast(DestAddr.getPointer(), Int8PtrTy), + DestAddr.getAlignment()); + llvm::Value *DestAddress = DestAddr.getPointer(); + + // Operands are Base, Dest, Modifier. + // The intrinsic format in LLVM IR is defined as + // { ValueType, i8* } (i8*, i32). + Ops = {BaseAddress, EmitScalarExpr(E->getArg(2))}; + + llvm::Value *Result = Builder.CreateCall(CGM.getIntrinsic(IntID), Ops); + // The value needs to be stored as the variable is passed by reference. + llvm::Value *DestVal = Builder.CreateExtractValue(Result, 0); + + // The store needs to be truncated to fit the destination type. + // While i32 and i64 are natively supported on Hexagon, i8 and i16 needs + // to be handled with stores of respective destination type. + DestVal = Builder.CreateTrunc(DestVal, DestTy); + + llvm::Value *DestForStore = + Builder.CreateBitCast(DestAddress, DestVal->getType()->getPointerTo()); + Builder.CreateAlignedStore(DestVal, DestForStore, DestAddr.getAlignment()); + // The updated value of the base pointer is returned. + return Builder.CreateExtractValue(Result, 1); + }; + switch (BuiltinID) { case Hexagon::BI__builtin_HEXAGON_V6_vaddcarry: case Hexagon::BI__builtin_HEXAGON_V6_vaddcarry_128B: { @@ -10909,6 +10948,18 @@ Value *CodeGenFunction::EmitHexagonBuiltinExpr(unsigned BuiltinID, return MakeCircSt(Intrinsic::hexagon_S2_storeri_pcr, /*HasImm=*/false); case Hexagon::BI__builtin_HEXAGON_S2_storerd_pcr: return MakeCircSt(Intrinsic::hexagon_S2_storerd_pcr, /*HasImm=*/false); + case Hexagon::BI__builtin_brev_ldub: + return MakeBrevLd(Intrinsic::hexagon_L2_loadrub_pbr, Int8Ty); + case Hexagon::BI__builtin_brev_ldb: + return MakeBrevLd(Intrinsic::hexagon_L2_loadrb_pbr, Int8Ty); + case Hexagon::BI__builtin_brev_lduh: + return MakeBrevLd(Intrinsic::hexagon_L2_loadruh_pbr, Int16Ty); + case Hexagon::BI__builtin_brev_ldh: + return MakeBrevLd(Intrinsic::hexagon_L2_loadrh_pbr, Int16Ty); + case Hexagon::BI__builtin_brev_ldw: + return MakeBrevLd(Intrinsic::hexagon_L2_loadri_pbr, Int32Ty); + case Hexagon::BI__builtin_brev_ldd: + return MakeBrevLd(Intrinsic::hexagon_L2_loadrd_pbr, Int64Ty); default: break; } // switch diff --git a/test/CodeGen/builtins-hexagon.c b/test/CodeGen/builtins-hexagon.c index 22835a23f2..3b4d8d057a 100644 --- a/test/CodeGen/builtins-hexagon.c +++ b/test/CodeGen/builtins-hexagon.c @@ -3322,28 +3322,30 @@ void test() { __builtin_HEXAGON_Y4_l2fetch(0, 0); // CHECK: @llvm.hexagon.Y5.l2fetch __builtin_HEXAGON_Y5_l2fetch(0, 0); - // CHECK: @llvm.hexagon.brev.ldb + + // CHECK: @llvm.hexagon.L2.loadrb.pbr __builtin_brev_ldb(0, 0, 0); - // CHECK: @llvm.hexagon.brev.ldd + // CHECK: @llvm.hexagon.L2.loadrd.pbr __builtin_brev_ldd(0, 0, 0); - // CHECK: @llvm.hexagon.brev.ldh + // CHECK: @llvm.hexagon.L2.loadrh.pbr __builtin_brev_ldh(0, 0, 0); - // CHECK: @llvm.hexagon.brev.ldub + // CHECK: @llvm.hexagon.L2.loadrub.pbr __builtin_brev_ldub(0, 0, 0); - // CHECK: @llvm.hexagon.brev.lduh + // CHECK: @llvm.hexagon.L2.loadruh.pbr __builtin_brev_lduh(0, 0, 0); - // CHECK: @llvm.hexagon.brev.ldw + // CHECK: @llvm.hexagon.L2.loadri.pbr __builtin_brev_ldw(0, 0, 0); - // CHECK: @llvm.hexagon.brev.stb + // CHECK: @llvm.hexagon.S2.storerb.pbr __builtin_brev_stb(0, 0, 0); - // CHECK: @llvm.hexagon.brev.std - __builtin_brev_std(0, 0, 0); - // CHECK: @llvm.hexagon.brev.sth + // CHECK: @llvm.hexagon.S2.storerd.pbr + __builtin_brev_std(0, 0LL, 0); + // CHECK: @llvm.hexagon.S2.storerh.pbr __builtin_brev_sth(0, 0, 0); - // CHECK: @llvm.hexagon.brev.sthhi + // CHECK: @llvm.hexagon.S2.storerf.pbr __builtin_brev_sthhi(0, 0, 0); - // CHECK: @llvm.hexagon.brev.stw + // CHECK: @llvm.hexagon.S2.storeri.pbr __builtin_brev_stw(0, 0, 0); + // CHECK: @llvm.hexagon.circ.ldb __builtin_circ_ldb(0, 0, 0, 0); // CHECK: @llvm.hexagon.circ.ldd diff --git a/test/CodeGen/hexagon-brev-ld-ptr-incdec.c b/test/CodeGen/hexagon-brev-ld-ptr-incdec.c new file mode 100644 index 0000000000..67c6c347e9 --- /dev/null +++ b/test/CodeGen/hexagon-brev-ld-ptr-incdec.c @@ -0,0 +1,52 @@ +// REQUIRES: hexagon-registered-target +// RUN: %clang_cc1 -emit-llvm -O2 -o - -triple hexagon-unknown-elf %s | FileCheck %s + +// The return value should return the value in A[1]. +// Check that the HexagonBuiltinExpr doesn't evaluate &(*ptr++) twice. If so, +// the return value will be the value in A[2] +// CHECK: @brev_ptr_inc +// CHECK-DAG: llvm.hexagon.L2.loadri.pbr +// CHECK-DAG: getelementptr{{.*}}i32 1 +// CHECK-NOT: getelementptr{{.*}}i32 2 +// CHECK-NOT: getelementptr{{.*}}i32 1 +int brev_ptr_inc(int A[], int B[]) { + int *p0 = &B[0]; + int *p1 = &A[0]; + __builtin_brev_ldw(p0, &*p1++, 8); + return (*p1); +} + +// The return value should return the value in A[0]. +// CHECK: @brev_ptr_dec +// CHECK: llvm.hexagon.L2.loadri.pbr +// CHECK: [[RET:%[0-9]+]] = load{{.*}}%A +// CHECK: ret{{.*}}[[RET]] +int brev_ptr_dec(int A[], int B[]) { + int *p0 = &B[0]; + int *p1 = &A[1]; + __builtin_brev_ldw(p0, &*p1--, 8); + return (*p1); +} + +// The store in bitcode needs to be of width correspondng to 16-bit. +// CHECK: @brev_ptr_half +// CHECK: llvm.hexagon.L2.loadrh.pbr +// CHECK: store{{.*}}i16{{.*}}i16* +short int brev_ptr_half(short int A[], short int B[]) { + short int *p0 = &B[0]; + short int *p1 = &A[0]; + __builtin_brev_ldh(p0, &*p1++, 8); + return (*p1); +} + +// The store in bitcode needs to be of width correspondng to 8-bit. +// CHECK: @brev_ptr_byte +// CHECK: llvm.hexagon.L2.loadrub.pbr +// CHECK: store{{.*}}i8{{.*}}i8* +unsigned char brev_ptr_byte(unsigned char A[], unsigned char B[]) { + unsigned char *p0 = &B[0]; + unsigned char *p1 = &A[0]; + __builtin_brev_ldub(p0, &*p1++, 8); + return (*p1); +} + diff --git a/test/CodeGen/hexagon-brev-store-elm.c b/test/CodeGen/hexagon-brev-store-elm.c new file mode 100644 index 0000000000..f593419e63 --- /dev/null +++ b/test/CodeGen/hexagon-brev-store-elm.c @@ -0,0 +1,46 @@ +// REQUIRES: hexagon-registered-target +// RUN: %clang_cc1 -emit-llvm -O2 -o - -triple hexagon-unknown-elf %s | FileCheck %s +// This unit test validates that the store to "dst" variable needs to be eliminated. + +// CHECK: @brev_store_elimination_test1 +// CHECK: llvm.hexagon.L2.loadri.pbr +// CHECK-NOT: store + +int *brev_store_elimination_test1(int *ptr, int mod) { + int dst = 100; + return __builtin_brev_ldw(ptr, &dst, mod); +} + +// CHECK: @brev_store_elimination_test2 +// CHECK: llvm.hexagon.L2.loadri.pbr +// CHECK-NOT: store +extern int add(int a); +int brev_store_elimination_test2(int *ptr, int mod) { + int dst = 100; + __builtin_brev_ldw(ptr, &dst, mod); + return add(dst); +} + +// CHECK: @brev_store_elimination_test3 +// CHECK: llvm.hexagon.L2.loadri.pbr +// CHECK-NOT: store +int brev_store_elimination_test3(int *ptr, int mod, int inc) { + int dst = 100; + for (int i = 0; i < inc; ++i) { + __builtin_brev_ldw(ptr, &dst, mod); + dst = add(dst); + } + return dst; +} + +// brev_store_elimination_test4 validates the fact that we are not deleting the +// stores if the value is passed by reference later. +// CHECK: @brev_store_elimination_test4 +// CHECK: llvm.hexagon.L2.loadri.pbr +// CHECK: store +extern int sub(int *a); +int brev_store_elimination_test4(int *ptr, int mod) { + int dst = 100; + __builtin_brev_ldw(ptr, &dst, mod); + return sub(&dst); +} -- 2.40.0