llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-llvm-selectiondag Author: jpwang (jjppp) <details> <summary>Changes</summary> Fixes #<!-- -->201448 This PR fixes downcast failures in lowering `__builtin_prefetch` for immediate arguments. The code currently assumes that the immediate operands are `ConstantInt`s. This is not always true as poison values may come from UB expressions (e.g., `__builtin_prefetch(0, 2 >> 32)`). This would cause the downcast `cast<ConstantInt>` to fail. Since the source program already contains UB, this PR avoids downcast failure by replacing poisonous values with a legal default value `0` in CodeGen. A regression test is also added to cover the poison immediate argument case. --- Patch is 89.77 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/201623.diff 2 Files Affected: - (added) clang/test/CodeGen/prefetch-poison-rw.c (+6) - (modified) llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp (+511-454) ``````````diff diff --git a/clang/test/CodeGen/prefetch-poison-rw.c b/clang/test/CodeGen/prefetch-poison-rw.c new file mode 100644 index 0000000000000..c13d4bb246d54 --- /dev/null +++ b/clang/test/CodeGen/prefetch-poison-rw.c @@ -0,0 +1,6 @@ +// RUN: %clang_cc1 -triple x86_64-pc-linux -emit-llvm %s -o - | FileCheck %s + +void test_poison_rw(void) { + __builtin_prefetch(0, 2 >> 32); + // CHECK: call void @llvm.prefetch.p0(ptr null, i32 0, i32 3, i32 1) +} diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp index eca5bb1598ae0..ff55e3862610a 100644 --- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp +++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp @@ -190,11 +190,12 @@ getCopyFromParts(SelectionDAG &DAG, const SDLoc &DL, const SDValue *Parts, // Assemble the power of 2 part. unsigned RoundParts = llvm::bit_floor(NumParts); unsigned RoundBits = PartBits * RoundParts; - EVT RoundVT = RoundBits == ValueBits ? - ValueVT : EVT::getIntegerVT(*DAG.getContext(), RoundBits); + EVT RoundVT = RoundBits == ValueBits + ? ValueVT + : EVT::getIntegerVT(*DAG.getContext(), RoundBits); SDValue Lo, Hi; - EVT HalfVT = EVT::getIntegerVT(*DAG.getContext(), RoundBits/2); + EVT HalfVT = EVT::getIntegerVT(*DAG.getContext(), RoundBits / 2); if (RoundParts > 2) { Lo = getCopyFromParts(DAG, DL, Parts, RoundParts / 2, PartVT, HalfVT, V, @@ -262,7 +263,7 @@ getCopyFromParts(SelectionDAG &DAG, const SDLoc &DL, const SDValue *Parts, ValueVT.bitsLT(PartEVT)) { // For an FP value in an integer part, we need to truncate to the right // width first. - PartEVT = EVT::getIntegerVT(*DAG.getContext(), ValueVT.getSizeInBits()); + PartEVT = EVT::getIntegerVT(*DAG.getContext(), ValueVT.getSizeInBits()); Val = DAG.getNode(ISD::TRUNCATE, DL, PartEVT, Val); } @@ -277,8 +278,8 @@ getCopyFromParts(SelectionDAG &DAG, const SDLoc &DL, const SDValue *Parts, // indicate whether the truncated bits will always be // zero or sign-extension. if (AssertOp) - Val = DAG.getNode(*AssertOp, DL, PartEVT, Val, - DAG.getValueType(ValueVT)); + Val = + DAG.getNode(*AssertOp, DL, PartEVT, Val, DAG.getValueType(ValueVT)); return DAG.getNode(ISD::TRUNCATE, DL, ValueVT, Val); } return DAG.getNode(ISD::ANY_EXTEND, DL, ValueVT, Val); @@ -368,7 +369,7 @@ static SDValue getCopyFromPartsVector(SelectionDAG &DAG, const SDLoc &DL, NumParts = NumRegs; // Silence a compiler warning. assert(RegisterVT == PartVT && "Part type doesn't match vector breakdown!"); assert(RegisterVT.getSizeInBits() == - Parts[0].getSimpleValueType().getSizeInBits() && + Parts[0].getSimpleValueType().getSizeInBits() && "Part type sizes don't match!"); // Assemble the parts into intermediate operands. @@ -451,21 +452,21 @@ static SDValue getCopyFromPartsVector(SelectionDAG &DAG, const SDLoc &DL, return DAG.getNode(ISD::BITCAST, DL, ValueVT, Val); if (ValueVT.getVectorNumElements() != 1) { - // Certain ABIs require that vectors are passed as integers. For vectors - // are the same size, this is an obvious bitcast. - if (ValueVT.getSizeInBits() == PartEVT.getSizeInBits()) { - return DAG.getNode(ISD::BITCAST, DL, ValueVT, Val); - } else if (ValueVT.bitsLT(PartEVT)) { - const uint64_t ValueSize = ValueVT.getFixedSizeInBits(); - EVT IntermediateType = EVT::getIntegerVT(*DAG.getContext(), ValueSize); - // Drop the extra bits. - Val = DAG.getNode(ISD::TRUNCATE, DL, IntermediateType, Val); - return DAG.getBitcast(ValueVT, Val); - } - - diagnosePossiblyInvalidConstraint( - *DAG.getContext(), V, "non-trivial scalar-to-vector conversion"); - return DAG.getUNDEF(ValueVT); + // Certain ABIs require that vectors are passed as integers. For vectors + // are the same size, this is an obvious bitcast. + if (ValueVT.getSizeInBits() == PartEVT.getSizeInBits()) { + return DAG.getNode(ISD::BITCAST, DL, ValueVT, Val); + } else if (ValueVT.bitsLT(PartEVT)) { + const uint64_t ValueSize = ValueVT.getFixedSizeInBits(); + EVT IntermediateType = EVT::getIntegerVT(*DAG.getContext(), ValueSize); + // Drop the extra bits. + Val = DAG.getNode(ISD::TRUNCATE, DL, IntermediateType, Val); + return DAG.getBitcast(ValueVT, Val); + } + + diagnosePossiblyInvalidConstraint( + *DAG.getContext(), V, "non-trivial scalar-to-vector conversion"); + return DAG.getUNDEF(ValueVT); } // Handle cases such as i8 -> <1 x i1> @@ -542,12 +543,11 @@ getCopyToParts(SelectionDAG &DAG, const SDLoc &DL, SDValue Val, SDValue *Parts, if (ValueVT.isFloatingPoint()) { // FP values need to be bitcast, then extended if they are being put // into a larger container. - ValueVT = EVT::getIntegerVT(*DAG.getContext(), ValueVT.getSizeInBits()); + ValueVT = EVT::getIntegerVT(*DAG.getContext(), ValueVT.getSizeInBits()); Val = DAG.getNode(ISD::BITCAST, DL, ValueVT, Val); } assert((PartVT.isInteger() || PartVT == MVT::x86mmx) && - ValueVT.isInteger() && - "Unknown mismatch!"); + ValueVT.isInteger() && "Unknown mismatch!"); ValueVT = EVT::getIntegerVT(*DAG.getContext(), NumParts * PartBits); Val = DAG.getNode(ExtendKind, DL, ValueVT, Val); if (PartVT == MVT::x86mmx) @@ -560,8 +560,7 @@ getCopyToParts(SelectionDAG &DAG, const SDLoc &DL, SDValue Val, SDValue *Parts, } else if (NumParts * PartBits < ValueVT.getSizeInBits()) { // If the parts cover less bits than value has, truncate the value. assert((PartVT.isInteger() || PartVT == MVT::x86mmx) && - ValueVT.isInteger() && - "Unknown mismatch!"); + ValueVT.isInteger() && "Unknown mismatch!"); ValueVT = EVT::getIntegerVT(*DAG.getContext(), NumParts * PartBits); Val = DAG.getNode(ISD::TRUNCATE, DL, ValueVT, Val); if (PartVT == MVT::x86mmx) @@ -592,8 +591,9 @@ getCopyToParts(SelectionDAG &DAG, const SDLoc &DL, SDValue Val, SDValue *Parts, unsigned RoundParts = llvm::bit_floor(NumParts); unsigned RoundBits = RoundParts * PartBits; unsigned OddParts = NumParts - RoundParts; - SDValue OddVal = DAG.getNode(ISD::SRL, DL, ValueVT, Val, - DAG.getShiftAmountConstant(RoundBits, ValueVT, DL)); + SDValue OddVal = + DAG.getNode(ISD::SRL, DL, ValueVT, Val, + DAG.getShiftAmountConstant(RoundBits, ValueVT, DL)); getCopyToParts(DAG, DL, OddVal, Parts + RoundParts, OddParts, PartVT, V, CallConv); @@ -609,22 +609,21 @@ getCopyToParts(SelectionDAG &DAG, const SDLoc &DL, SDValue Val, SDValue *Parts, // The number of parts is a power of 2. Repeatedly bisect the value using // EXTRACT_ELEMENT. - Parts[0] = DAG.getNode(ISD::BITCAST, DL, - EVT::getIntegerVT(*DAG.getContext(), - ValueVT.getSizeInBits()), - Val); + Parts[0] = DAG.getNode( + ISD::BITCAST, DL, + EVT::getIntegerVT(*DAG.getContext(), ValueVT.getSizeInBits()), Val); for (unsigned StepSize = NumParts; StepSize > 1; StepSize /= 2) { for (unsigned i = 0; i < NumParts; i += StepSize) { unsigned ThisBits = StepSize * PartBits / 2; EVT ThisVT = EVT::getIntegerVT(*DAG.getContext(), ThisBits); SDValue &Part0 = Parts[i]; - SDValue &Part1 = Parts[i+StepSize/2]; + SDValue &Part1 = Parts[i + StepSize / 2]; - Part1 = DAG.getNode(ISD::EXTRACT_ELEMENT, DL, - ThisVT, Part0, DAG.getIntPtrConstant(1, DL)); - Part0 = DAG.getNode(ISD::EXTRACT_ELEMENT, DL, - ThisVT, Part0, DAG.getIntPtrConstant(0, DL)); + Part1 = DAG.getNode(ISD::EXTRACT_ELEMENT, DL, ThisVT, Part0, + DAG.getIntPtrConstant(1, DL)); + Part0 = DAG.getNode(ISD::EXTRACT_ELEMENT, DL, ThisVT, Part0, + DAG.getIntPtrConstant(0, DL)); if (ThisBits == PartBits && ThisVT != PartVT) { Part0 = DAG.getNode(ISD::BITCAST, DL, PartVT, Part0); @@ -915,9 +914,9 @@ SDValue RegsForValue::getCopyFromRegs(SelectionDAG &DAG, for (unsigned i = 0; i != NumRegs; ++i) { SDValue P; if (!Glue) { - P = DAG.getCopyFromReg(Chain, dl, Regs[Part+i], RegisterVT); + P = DAG.getCopyFromReg(Chain, dl, Regs[Part + i], RegisterVT); } else { - P = DAG.getCopyFromReg(Chain, dl, Regs[Part+i], RegisterVT, *Glue); + P = DAG.getCopyFromReg(Chain, dl, Regs[Part + i], RegisterVT, *Glue); *Glue = P.getValue(2); } @@ -930,7 +929,7 @@ SDValue RegsForValue::getCopyFromRegs(SelectionDAG &DAG, continue; const FunctionLoweringInfo::LiveOutInfo *LOI = - FuncInfo.GetLiveOutRegInfo(Regs[Part+i]); + FuncInfo.GetLiveOutRegInfo(Regs[Part + i]); if (!LOI) continue; @@ -1027,7 +1026,7 @@ void RegsForValue::getCopyToRegs(SDValue Val, SelectionDAG &DAG, // c3 = TokenFactor c1, c2 // ... // = op c3, ..., f2 - Chain = Chains[NumRegs-1]; + Chain = Chains[NumRegs - 1]; else Chain = DAG.getNode(ISD::TokenFactor, dl, MVT::Other, Chains); } @@ -1075,8 +1074,8 @@ void RegsForValue::AddInlineAsmOperands(InlineAsm::Kind Code, bool HasMatching, for (unsigned Value = 0, Reg = 0, e = ValueVTs.size(); Value != e; ++Value) { MVT RegisterVT = RegVTs[Value]; - unsigned NumRegs = TLI.getNumRegisters(*DAG.getContext(), ValueVTs[Value], - RegisterVT); + unsigned NumRegs = + TLI.getNumRegisters(*DAG.getContext(), ValueVTs[Value], RegisterVT); for (unsigned i = 0; i != NumRegs; ++i) { assert(Reg < Regs.size() && "Mismatch in # registers expected"); Register TheReg = Regs[Reg++]; @@ -1145,7 +1144,7 @@ SDValue SelectionDAGBuilder::updateRoot(SmallVectorImpl<SDValue> &Pending) { for (; i != e; ++i) { assert(Pending[i].getNode()->getNumOperands() > 1); if (Pending[i].getNode()->getOperand(0) == Root) - break; // Don't add the root if we already indirectly depend on it. + break; // Don't add the root if we already indirectly depend on it. } if (i == e) @@ -1207,11 +1206,9 @@ SDValue SelectionDAGBuilder::getRoot() { // Chain up all pending constrained intrinsics together with all // pending loads, by simply appending them to PendingLoads and // then calling getMemoryRoot(). - PendingLoads.reserve(PendingLoads.size() + - PendingConstrainedFP.size() + + PendingLoads.reserve(PendingLoads.size() + PendingConstrainedFP.size() + PendingConstrainedFPStrict.size()); - PendingLoads.append(PendingConstrainedFP.begin(), - PendingConstrainedFP.end()); + PendingLoads.append(PendingConstrainedFP.begin(), PendingConstrainedFP.end()); PendingLoads.append(PendingConstrainedFPStrict.begin(), PendingConstrainedFPStrict.end()); PendingConstrainedFP.clear(); @@ -1425,10 +1422,13 @@ void SelectionDAGBuilder::visit(unsigned Opcode, const User &I) { // Note: this doesn't use InstVisitor, because it has to work with // ConstantExpr's in addition to instructions. switch (Opcode) { - default: llvm_unreachable("Unknown instruction type encountered!"); + default: + llvm_unreachable("Unknown instruction type encountered!"); // Build the switch statement using the Instruction.def file. -#define HANDLE_INST(NUM, OPCODE, CLASS) \ - case Instruction::OPCODE: visit##OPCODE((const CLASS&)I); break; +#define HANDLE_INST(NUM, OPCODE, CLASS) \ + case Instruction::OPCODE: \ + visit##OPCODE((const CLASS &)I); \ + break; #include "llvm/IR/Instruction.def" } } @@ -1786,8 +1786,8 @@ SDValue SelectionDAGBuilder::getCopyFromRegs(const Value *V, Type *Ty) { DAG.getDataLayout(), InReg, Ty, std::nullopt); // This is not an ABI copy. SDValue Chain = DAG.getEntryNode(); - Result = RFV.getCopyFromRegs(DAG, FuncInfo, getCurSDLoc(), Chain, nullptr, - V); + Result = + RFV.getCopyFromRegs(DAG, FuncInfo, getCurSDLoc(), Chain, nullptr, V); resolveDanglingDebugInfo(V, Result); } @@ -1800,7 +1800,8 @@ SDValue SelectionDAGBuilder::getValue(const Value *V) { // to do this first, so that we don't create a CopyFromReg if we already // have a regular SDValue. SDValue &N = NodeMap[V]; - if (N.getNode()) return N; + if (N.getNode()) + return N; // If there's a virtual register allocated and initialized for this // value, use it. @@ -1903,7 +1904,8 @@ SDValue SelectionDAGBuilder::getValueImpl(const Value *V) { for (const Use &U : C->operands()) { SDNode *Val = getValue(U).getNode(); // If the operand is an empty aggregate, there are no values. - if (!Val) continue; + if (!Val) + continue; // Add each leaf value from the operand to the Constants list // to form a flattened list of all the values. for (unsigned i = 0, e = Val->getNumValues(); i != e; ++i) @@ -1914,7 +1916,7 @@ SDValue SelectionDAGBuilder::getValueImpl(const Value *V) { } if (const ConstantDataSequential *CDS = - dyn_cast<ConstantDataSequential>(C)) { + dyn_cast<ConstantDataSequential>(C)) { SmallVector<SDValue, 4> Ops; for (uint64_t i = 0, e = CDS->getNumElements(); i != e; ++i) { SDNode *Val = getValue(CDS->getElementAsConstant(i)).getNode(); @@ -2118,7 +2120,7 @@ static void findUnwindDestinations( SmallVectorImpl<std::pair<MachineBasicBlock *, BranchProbability>> &UnwindDests) { EHPersonality Personality = - classifyEHPersonality(FuncInfo.Fn->getPersonalityFn()); + classifyEHPersonality(FuncInfo.Fn->getPersonalityFn()); bool IsMSVCCXX = Personality == EHPersonality::MSVC_CXX; bool IsCoreCLR = Personality == EHPersonality::CoreCLR; bool IsWasmCXX = Personality == EHPersonality::Wasm_CXX; @@ -2245,8 +2247,7 @@ void SelectionDAGBuilder::visitRet(const ReturnInst &I) { commonAlignment(BaseAlign, Offsets[i])); } - Chain = DAG.getNode(ISD::TokenFactor, getCurSDLoc(), - MVT::Other, Chains); + Chain = DAG.getNode(ISD::TokenFactor, getCurSDLoc(), MVT::Other, Chains); } else if (I.getNumOperands() != 0) { SmallVector<Type *, 4> Types; ComputeValueTypes(DL, I.getOperand(0)->getType(), Types); @@ -2341,7 +2342,7 @@ void SelectionDAGBuilder::visitRet(const ReturnInst &I) { bool isVarArg = DAG.getMachineFunction().getFunction().isVarArg(); CallingConv::ID CallConv = - DAG.getMachineFunction().getFunction().getCallingConv(); + DAG.getMachineFunction().getFunction().getCallingConv(); Chain = DAG.getTargetLoweringInfo().LowerReturn( Chain, CallConv, isVarArg, Outs, OutVals, getCurSDLoc(), DAG); @@ -2374,17 +2375,19 @@ void SelectionDAGBuilder::CopyToExportRegsIfNeeded(const Value *V) { /// CopyTo/FromReg. void SelectionDAGBuilder::ExportFromCurrentBlock(const Value *V) { // No need to export constants. - if (!isa<Instruction>(V) && !isa<Argument>(V)) return; + if (!isa<Instruction>(V) && !isa<Argument>(V)) + return; // Already exported? - if (FuncInfo.isExportedInst(V)) return; + if (FuncInfo.isExportedInst(V)) + return; Register Reg = FuncInfo.InitializeRegForValue(V); CopyValueToVirtualRegister(V, Reg); } -bool SelectionDAGBuilder::isExportableFromCurrentBlock(const Value *V, - const BasicBlock *FromBB) { +bool SelectionDAGBuilder::isExportableFromCurrentBlock( + const Value *V, const BasicBlock *FromBB) { // The operands of the setcc have to be in this block. We don't know // how to export them from some other block. if (const Instruction *VI = dyn_cast<Instruction>(V)) { @@ -2447,15 +2450,10 @@ static bool InBlock(const Value *V, const BasicBlock *BB) { /// EmitBranchForMergedCondition - Helper method for FindMergedConditions. /// This function emits a branch and is used at the leaves of an OR or an /// AND operator tree. -void -SelectionDAGBuilder::EmitBranchForMergedCondition(const Value *Cond, - MachineBasicBlock *TBB, - MachineBasicBlock *FBB, - MachineBasicBlock *CurBB, - MachineBasicBlock *SwitchBB, - BranchProbability TProb, - BranchProbability FProb, - bool InvertCond) { +void SelectionDAGBuilder::EmitBranchForMergedCondition( + const Value *Cond, MachineBasicBlock *TBB, MachineBasicBlock *FBB, + MachineBasicBlock *CurBB, MachineBasicBlock *SwitchBB, + BranchProbability TProb, BranchProbability FProb, bool InvertCond) { const BasicBlock *BB = CurBB->getBasicBlock(); // If the leaf of the tree is a comparison, merge the condition into @@ -2494,8 +2492,8 @@ SelectionDAGBuilder::EmitBranchForMergedCondition(const Value *Cond, // Create a CaseBlock record representing this branch. ISD::CondCode Opc = InvertCond ? ISD::SETNE : ISD::SETEQ; - CaseBlock CB(Opc, Cond, ConstantInt::getTrue(*DAG.getContext()), - nullptr, TBB, FBB, CurBB, getCurSDLoc(), TProb, FProb); + CaseBlock CB(Opc, Cond, ConstantInt::getTrue(*DAG.getContext()), nullptr, TBB, + FBB, CurBB, getCurSDLoc(), TProb, FProb); SL->SwitchCases.push_back(CB); } @@ -2635,15 +2633,11 @@ bool SelectionDAGBuilder::shouldKeepJumpConditionsTogether( return true; } -void SelectionDAGBuilder::FindMergedConditions(const Value *Cond, - MachineBasicBlock *TBB, - MachineBasicBlock *FBB, - MachineBasicBlock *CurBB, - MachineBasicBlock *SwitchBB, - Instruction::BinaryOps Opc, - BranchProbability TProb, - BranchProbability FProb, - bool InvertCond) { +void SelectionDAGBuilder::FindMergedConditions( + const Value *Cond, MachineBasicBlock *TBB, MachineBasicBlock *FBB, + MachineBasicBlock *CurBB, MachineBasicBlock *SwitchBB, + Instruction::BinaryOps Opc, BranchProbability TProb, + BranchProbability FProb, bool InvertCond) { // Skip over not part of the tree and remember to invert op and operands at // next level. Value *NotCond; @@ -2682,8 +2676,8 @@ void SelectionDAGBuilder::FindMergedConditions(const Value *Cond, if (!BOpIsInOrAndTree || BOp->getParent() != CurBB->getBasicBlock() || !InBlock(BOpOp0, CurBB->getBasicBlock()) || !InBlock(BOpOp1, CurBB->getBasicBlock())) { - EmitBranchForMergedCondition(Cond, TBB, FBB, CurBB, SwitchBB, - TProb, FProb, InvertCond); + EmitBranchForMergedCondition(Cond, TBB, FBB, CurBB, SwitchBB, TProb, FProb, + InvertCond); return; } @@ -2765,9 +2759,10 @@ void SelectionDAGBuilder::FindMergedConditions(const Value *Cond, /// If the set of cases should be emitted as a series of branches, return true. /// If we should emit this as a bunch of and/or'd together conditions, return /// false. -bool -SelectionDAGBuilder::ShouldEmitAsBranches(const std::vector<CaseBlock> &Cases) { - if (Cases.size() != 2) return true; +bool SelectionDAGBuilder::ShouldEmitAsBranches( + const std::vector<CaseBlock> &Cases) { + if (Cases.size() != 2) + return true; // If this is two comparisons of the same values or'd or and'd ... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/201623 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
