Skip to content

Commit

Permalink
Reapply: [Float2Int] Resolve FIXME: Pick the smallest legal type that…
Browse files Browse the repository at this point in the history
… fits (#86337)

Originally reverted because of a bug in Range that is now fixed
(#86041), we can reland this commit. Tests have been added to ensure the
miscompile that caused the revert does not happen again.
  • Loading branch information
AtariDreams committed Mar 25, 2024
1 parent a83ed04 commit 7b3e943
Show file tree
Hide file tree
Showing 4 changed files with 334 additions and 69 deletions.
2 changes: 1 addition & 1 deletion llvm/include/llvm/Transforms/Scalar/Float2Int.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ class Float2IntPass : public PassInfoMixin<Float2IntPass> {
std::optional<ConstantRange> calcRange(Instruction *I);
void walkBackwards();
void walkForwards();
bool validateAndTransform();
bool validateAndTransform(const DataLayout &DL);
Value *convert(Instruction *I, Type *ToTy);
void cleanup();

Expand Down
29 changes: 19 additions & 10 deletions llvm/lib/Transforms/Scalar/Float2Int.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,7 @@ void Float2IntPass::walkForwards() {
}

// If there is a valid transform to be done, do it.
bool Float2IntPass::validateAndTransform() {
bool Float2IntPass::validateAndTransform(const DataLayout &DL) {
bool MadeChange = false;

// Iterate over every disjoint partition of the def-use graph.
Expand Down Expand Up @@ -374,15 +374,23 @@ bool Float2IntPass::validateAndTransform() {
LLVM_DEBUG(dbgs() << "F2I: Value not guaranteed to be representable!\n");
continue;
}
if (MinBW > 64) {
LLVM_DEBUG(
dbgs() << "F2I: Value requires more than 64 bits to represent!\n");
continue;
}

// OK, R is known to be representable. Now pick a type for it.
// FIXME: Pick the smallest legal type that will fit.
Type *Ty = (MinBW > 32) ? Type::getInt64Ty(*Ctx) : Type::getInt32Ty(*Ctx);
// OK, R is known to be representable.
// Pick the smallest legal type that will fit.
Type *Ty = DL.getSmallestLegalIntType(*Ctx, MinBW);
if (!Ty) {
// Every supported target supports 64-bit and 32-bit integers,
// so fallback to a 32 or 64-bit integer if the value fits.
if (MinBW <= 32) {
Ty = Type::getInt32Ty(*Ctx);
} else if (MinBW <= 64) {
Ty = Type::getInt64Ty(*Ctx);
} else {
LLVM_DEBUG(dbgs() << "F2I: Value requires more bits to represent than "
"the target supports!\n");
continue;
}
}

for (auto MI = ECs.member_begin(It), ME = ECs.member_end();
MI != ME; ++MI)
Expand Down Expand Up @@ -489,7 +497,8 @@ bool Float2IntPass::runImpl(Function &F, const DominatorTree &DT) {
walkBackwards();
walkForwards();

bool Modified = validateAndTransform();
const DataLayout &DL = F.getParent()->getDataLayout();
bool Modified = validateAndTransform(DL);
if (Modified)
cleanup();
return Modified;
Expand Down

0 comments on commit 7b3e943

Please sign in to comment.