peterwaller-arm updated this revision to Diff 311612. peterwaller-arm added a comment.
I've taken over the differential. Simplify things a bit more, remove an untested remnant created from before the patch split. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91806/new/ https://reviews.llvm.org/D91806 Files: llvm/lib/Transforms/Utils/Local.cpp llvm/test/Transforms/InstCombine/debuginfo-scalable-typesize.ll Index: llvm/test/Transforms/InstCombine/debuginfo-scalable-typesize.ll =================================================================== --- /dev/null +++ llvm/test/Transforms/InstCombine/debuginfo-scalable-typesize.ll @@ -0,0 +1,35 @@ +; RUN: opt -instcombine -S < %s 2>%t | FileCheck %s +; RUN: FileCheck --check-prefix=ERR --allow-empty %s <%t + +; This test is defending against a TypeSize message raised in the method +; `valueCoversEntireFragment` in Local.cpp because of an implicit cast from +; `TypeSize` to `uint64_t`. This particular TypeSize message only occurred when +; debug info was available. + +; If this check fails please read +; clang/test/CodeGen/aarch64-sve-intrinsics/README for instructions on +; how to resolve it. + +; ERR-NOT: TypeSize is not scalable + +; CHECK-LABEL: @debug_local_scalable( +define <vscale x 2 x double> @debug_local_scalable(<vscale x 2 x double> %tostore) { + %vx = alloca <vscale x 2 x double>, align 16 + call void @llvm.dbg.declare(metadata <vscale x 2 x double>* %vx, metadata !3, metadata !DIExpression()), !dbg !5 + store <vscale x 2 x double> %tostore, <vscale x 2 x double>* %vx, align 16 + %ret = call <vscale x 2 x double> @f(<vscale x 2 x double>* %vx) + ret <vscale x 2 x double> %ret +} + +declare <vscale x 2 x double> @f(<vscale x 2 x double>*) + +declare void @llvm.dbg.declare(metadata, metadata, metadata) + +!llvm.module.flags = !{!2} + +!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1) +!1 = !DIFile(filename: "/tmp/test.c", directory: "/tmp/") +!2 = !{i32 2, !"Debug Info Version", i32 3} +!3 = !DILocalVariable(scope: !4) +!4 = distinct !DISubprogram(unit: !0) +!5 = !DILocation(scope: !4) Index: llvm/lib/Transforms/Utils/Local.cpp =================================================================== --- llvm/lib/Transforms/Utils/Local.cpp +++ llvm/lib/Transforms/Utils/Local.cpp @@ -1368,16 +1368,22 @@ /// least n bits. static bool valueCoversEntireFragment(Type *ValTy, DbgVariableIntrinsic *DII) { const DataLayout &DL = DII->getModule()->getDataLayout(); - uint64_t ValueSize = DL.getTypeAllocSizeInBits(ValTy); - if (auto FragmentSize = DII->getFragmentSizeInBits()) - return ValueSize >= *FragmentSize; + TypeSize ValueSize = DL.getTypeAllocSizeInBits(ValTy); + if (Optional<uint64_t> FragmentSize = DII->getFragmentSizeInBits()) { + assert(!ValueSize.isScalable() && + "Fragments don't work on scalable types."); + return ValueSize.getFixedSize() >= *FragmentSize; + } // We can't always calculate the size of the DI variable (e.g. if it is a // VLA). Try to use the size of the alloca that the dbg intrinsic describes // intead. if (DII->isAddressOfVariable()) if (auto *AI = dyn_cast_or_null<AllocaInst>(DII->getVariableLocation())) - if (auto FragmentSize = AI->getAllocationSizeInBits(DL)) - return ValueSize >= *FragmentSize; + if (Optional<TypeSize> FragmentSize = AI->getAllocationSizeInBits(DL)) { + assert(ValueSize.isScalable() == FragmentSize->isScalable() && + "Both sizes should agree on the scalable flag."); + return TypeSize::isKnownGE(ValueSize, *FragmentSize); + } // Could not determine size of variable. Conservatively return false. return false; }
Index: llvm/test/Transforms/InstCombine/debuginfo-scalable-typesize.ll =================================================================== --- /dev/null +++ llvm/test/Transforms/InstCombine/debuginfo-scalable-typesize.ll @@ -0,0 +1,35 @@ +; RUN: opt -instcombine -S < %s 2>%t | FileCheck %s +; RUN: FileCheck --check-prefix=ERR --allow-empty %s <%t + +; This test is defending against a TypeSize message raised in the method +; `valueCoversEntireFragment` in Local.cpp because of an implicit cast from +; `TypeSize` to `uint64_t`. This particular TypeSize message only occurred when +; debug info was available. + +; If this check fails please read +; clang/test/CodeGen/aarch64-sve-intrinsics/README for instructions on +; how to resolve it. + +; ERR-NOT: TypeSize is not scalable + +; CHECK-LABEL: @debug_local_scalable( +define <vscale x 2 x double> @debug_local_scalable(<vscale x 2 x double> %tostore) { + %vx = alloca <vscale x 2 x double>, align 16 + call void @llvm.dbg.declare(metadata <vscale x 2 x double>* %vx, metadata !3, metadata !DIExpression()), !dbg !5 + store <vscale x 2 x double> %tostore, <vscale x 2 x double>* %vx, align 16 + %ret = call <vscale x 2 x double> @f(<vscale x 2 x double>* %vx) + ret <vscale x 2 x double> %ret +} + +declare <vscale x 2 x double> @f(<vscale x 2 x double>*) + +declare void @llvm.dbg.declare(metadata, metadata, metadata) + +!llvm.module.flags = !{!2} + +!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1) +!1 = !DIFile(filename: "/tmp/test.c", directory: "/tmp/") +!2 = !{i32 2, !"Debug Info Version", i32 3} +!3 = !DILocalVariable(scope: !4) +!4 = distinct !DISubprogram(unit: !0) +!5 = !DILocation(scope: !4) Index: llvm/lib/Transforms/Utils/Local.cpp =================================================================== --- llvm/lib/Transforms/Utils/Local.cpp +++ llvm/lib/Transforms/Utils/Local.cpp @@ -1368,16 +1368,22 @@ /// least n bits. static bool valueCoversEntireFragment(Type *ValTy, DbgVariableIntrinsic *DII) { const DataLayout &DL = DII->getModule()->getDataLayout(); - uint64_t ValueSize = DL.getTypeAllocSizeInBits(ValTy); - if (auto FragmentSize = DII->getFragmentSizeInBits()) - return ValueSize >= *FragmentSize; + TypeSize ValueSize = DL.getTypeAllocSizeInBits(ValTy); + if (Optional<uint64_t> FragmentSize = DII->getFragmentSizeInBits()) { + assert(!ValueSize.isScalable() && + "Fragments don't work on scalable types."); + return ValueSize.getFixedSize() >= *FragmentSize; + } // We can't always calculate the size of the DI variable (e.g. if it is a // VLA). Try to use the size of the alloca that the dbg intrinsic describes // intead. if (DII->isAddressOfVariable()) if (auto *AI = dyn_cast_or_null<AllocaInst>(DII->getVariableLocation())) - if (auto FragmentSize = AI->getAllocationSizeInBits(DL)) - return ValueSize >= *FragmentSize; + if (Optional<TypeSize> FragmentSize = AI->getAllocationSizeInBits(DL)) { + assert(ValueSize.isScalable() == FragmentSize->isScalable() && + "Both sizes should agree on the scalable flag."); + return TypeSize::isKnownGE(ValueSize, *FragmentSize); + } // Could not determine size of variable. Conservatively return false. return false; }
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits