From ffab2f9805185189e50301e6ede3d019790830bb Mon Sep 17 00:00:00 2001 From: Gabriel Baraldi Date: Thu, 23 Nov 2023 15:46:24 -0300 Subject: [PATCH 1/2] Backport https://github.com/llvm/llvm-project/commit/5c3beb7b1e26d38b0933a28432dfbce4e00cf329 (#22) to avoid non integral addresspace issues --- llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp | 11 ++++++++--- llvm/test/Transforms/MemCpyOpt/memcpy.ll | 9 +++++++++ 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp b/llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp index a95d6adf36d6dd..ceee2cb5b877b1 100644 --- a/llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp +++ b/llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp @@ -1427,6 +1427,13 @@ bool MemCpyOptPass::processMemCpy(MemCpyInst *M, BasicBlock::iterator &BBI) { eraseInstruction(M); return true; } + // If the size is zero, remove the memcpy. This also prevents infinite loops + // in processMemSetMemCpyDependence, which is a no-op for zero-length memcpys. + + MemoryUseOrDef *MA = MSSA->getMemoryAccess(M); + if (!MA) + // Degenerate case: memcpy marked as not accessing memory. + return false; // If copying from a constant, try to turn the memcpy into a memset. if (auto *GV = dyn_cast(M->getSource())) @@ -1436,8 +1443,7 @@ bool MemCpyOptPass::processMemCpy(MemCpyInst *M, BasicBlock::iterator &BBI) { IRBuilder<> Builder(M); Instruction *NewM = Builder.CreateMemSet( M->getRawDest(), ByteVal, M->getLength(), M->getDestAlign(), false); - auto *LastDef = - cast(MSSAU->getMemorySSA()->getMemoryAccess(M)); + auto *LastDef = cast(MA); auto *NewAccess = MSSAU->createMemoryAccessAfter(NewM, LastDef, LastDef); MSSAU->insertDef(cast(NewAccess), /*RenameUses=*/true); @@ -1448,7 +1454,6 @@ bool MemCpyOptPass::processMemCpy(MemCpyInst *M, BasicBlock::iterator &BBI) { } BatchAAResults BAA(*AA); - MemoryUseOrDef *MA = MSSA->getMemoryAccess(M); // FIXME: Not using getClobberingMemoryAccess() here due to PR54682. MemoryAccess *AnyClobber = MA->getDefiningAccess(); MemoryLocation DestLoc = MemoryLocation::getForDest(M); diff --git a/llvm/test/Transforms/MemCpyOpt/memcpy.ll b/llvm/test/Transforms/MemCpyOpt/memcpy.ll index 7488203d5db16d..413d72a8e61155 100644 --- a/llvm/test/Transforms/MemCpyOpt/memcpy.ll +++ b/llvm/test/Transforms/MemCpyOpt/memcpy.ll @@ -723,6 +723,15 @@ define void @byval_param_noalias_metadata(ptr align 4 byval(i32) %ptr) { ret void } +define void @memcpy_memory_none(ptr %p, ptr %p2, i64 %size) { +; CHECK-LABEL: @memcpy_memory_none( +; CHECK-NEXT: call void @llvm.memcpy.p0.p0.i64(ptr [[P:%.*]], ptr [[P2:%.*]], i64 [[SIZE:%.*]], i1 false) #[[ATTR6:[0-9]+]] +; CHECK-NEXT: ret void +; + call void @llvm.memcpy.p0.p0.i64(ptr %p, ptr %p2, i64 %size, i1 false) memory(none) + ret void +} + !0 = !{!0} !1 = !{!1, !0} !2 = !{!1} From 11b73cce6d4a920193a7f1021f5fc7015c84265e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sergio=20S=C3=A1nchez=20Ram=C3=ADrez?= Date: Mon, 4 Mar 2024 19:48:31 +0100 Subject: [PATCH 2/2] Reapply "[MC] Always emit relocations for same-section function references" This reverts commit c0e2762e4008faa6f00da3e9d906413232a5b3bb. --- llvm/lib/MC/WinCOFFObjectWriter.cpp | 12 +++++++----- llvm/test/MC/COFF/diff.s | 25 +++++++++++++++++-------- 2 files changed, 24 insertions(+), 13 deletions(-) diff --git a/llvm/lib/MC/WinCOFFObjectWriter.cpp b/llvm/lib/MC/WinCOFFObjectWriter.cpp index b7b8692d53f316..c203280d2c107e 100644 --- a/llvm/lib/MC/WinCOFFObjectWriter.cpp +++ b/llvm/lib/MC/WinCOFFObjectWriter.cpp @@ -1190,12 +1190,14 @@ void WinCOFFObjectWriter::reset() { bool WinCOFFObjectWriter::isSymbolRefDifferenceFullyResolvedImpl( const MCAssembler &Asm, const MCSymbol &SymA, const MCFragment &FB, bool InSet, bool IsPCRel) const { - // MS LINK expects to be able to replace all references to a function with a - // thunk to implement their /INCREMENTAL feature. Make sure we don't optimize - // away any relocations to functions. + // Don't drop relocations between functions, even if they are in the same text + // section. Multiple Visual C++ linker features depend on having the + // relocations present. The /INCREMENTAL flag will cause these relocations to + // point to thunks, and the /GUARD:CF flag assumes that it can use relocations + // to approximate the set of all address taken functions. LLD's implementation + // of /GUARD:CF also relies on the existance of these relocations. uint16_t Type = cast(SymA).getType(); - if (Asm.isIncrementalLinkerCompatible() && - (Type >> COFF::SCT_COMPLEX_TYPE_SHIFT) == COFF::IMAGE_SYM_DTYPE_FUNCTION) + if ((Type >> COFF::SCT_COMPLEX_TYPE_SHIFT) == COFF::IMAGE_SYM_DTYPE_FUNCTION) return false; return MCObjectWriter::isSymbolRefDifferenceFullyResolvedImpl(Asm, SymA, FB, InSet, IsPCRel); diff --git a/llvm/test/MC/COFF/diff.s b/llvm/test/MC/COFF/diff.s index 640bf8189e0395..90466b59d02522 100644 --- a/llvm/test/MC/COFF/diff.s +++ b/llvm/test/MC/COFF/diff.s @@ -1,19 +1,14 @@ // RUN: llvm-mc -filetype=obj -triple i686-pc-mingw32 %s | llvm-readobj -S --sr --sd - | FileCheck %s +// COFF resolves differences between labels in the same section, unless that +// label is declared with function type. + .section baz, "xr" - .def X - .scl 2; - .type 32; - .endef .globl X X: mov Y-X+42, %eax retl - .def Y - .scl 2; - .type 32; - .endef .globl Y Y: retl @@ -30,6 +25,11 @@ _foobar: # @foobar # %bb.0: ret + .globl _baz +_baz: + calll _foobar + retl + .data .globl _rust_crate # @rust_crate .align 4 @@ -39,6 +39,15 @@ _rust_crate: .long _foobar-_rust_crate .long _foobar-_rust_crate +// Even though _baz and _foobar are in the same .text section, we keep the +// relocation for compatibility with the VC linker's /guard:cf and /incremental +// flags, even on mingw. + +// CHECK: Name: .text +// CHECK: Relocations [ +// CHECK-NEXT: 0x12 IMAGE_REL_I386_REL32 _foobar +// CHECK-NEXT: ] + // CHECK: Name: .data // CHECK: Relocations [ // CHECK-NEXT: 0x4 IMAGE_REL_I386_DIR32 _foobar