This is an archive of the discontinued LLVM Phabricator instance.

[PowerPC] Fix lrint and llrint code generation
AbandonedPublic

Authored by vddvss on Oct 21 2019, 10:20 AM.

Download Raw Diff

Details

Reviewers

hfinkel
nemanjai
stefanp
jsji

Summary

This patch fixes an issue where, in certain instances, when compiling code using the lrint and llrint LLVM intrinsics, LLVM can incorrectly optimize a loop to use hardware loop counters.

In terms of background, r361875 added lrint and llrint intrinsics to LLVM; however, the series only added support for instructions for AArch64, so on PPC and other platforms ll/lrint was emitted as a function call. On PPC, the hardware loop transform pass assumed that since an intrinsic was used, the ctr register would not be clobbered, and a transformation of the loop to use hardware loop intrinsics was valid. The function call clobbered ctr, resulting in incorrect code generation.

Diff Detail

Repository: rG LLVM Github Monorepo

Event Timeline

vddvss created this revision.Oct 21 2019, 10:20 AM

Herald added subscribers: llvm-commits, shchenz, MaskRay and 3 others. · View Herald TranscriptOct 21 2019, 10:20 AM

I believe this is already addressed in https://reviews.llvm.org/D68841 which is waiting for review.

Herald added a subscriber: • wuzish. · View Herald TranscriptOct 21 2019, 4:42 PM

Thanks. Should have checked :) This issue is causing buffer overflows in some applications compiled with Clang 9, so hopefully your fix can be ok'd soon.

https://reviews.llvm.org/D68841 has landed in https://reviews.llvm.org/rG97e36260709c541044f30092b420238511e13e5b. Does it fix your problem? If so, maybe you can abandon this revision. Thanks.

vddvss abandoned this revision.Nov 5 2019, 12:58 PM

Revision Contents

Path

Size

llvm/

lib/

Target/

PowerPC/

PPCTargetTransformInfo.cpp

2 lines

test/

CodeGen/

PowerPC/

llrint-ctr-crash.ll

171 lines

Diff 225907

llvm/lib/Target/PowerPC/PPCTargetTransformInfo.cpp

Show First 20 Lines • Show All 325 Lines • ▼ Show 20 Lines	#endif
return true;		return true;
else		else
continue; // ISD::FCOPYSIGN is never a library call.		continue; // ISD::FCOPYSIGN is never a library call.
case Intrinsic::sqrt: Opcode = ISD::FSQRT; break;		case Intrinsic::sqrt: Opcode = ISD::FSQRT; break;
case Intrinsic::floor: Opcode = ISD::FFLOOR; break;		case Intrinsic::floor: Opcode = ISD::FFLOOR; break;
case Intrinsic::ceil: Opcode = ISD::FCEIL; break;		case Intrinsic::ceil: Opcode = ISD::FCEIL; break;
case Intrinsic::trunc: Opcode = ISD::FTRUNC; break;		case Intrinsic::trunc: Opcode = ISD::FTRUNC; break;
case Intrinsic::rint: Opcode = ISD::FRINT; break;		case Intrinsic::rint: Opcode = ISD::FRINT; break;
		case Intrinsic::lrint: Opcode = ISD::LRINT; break;
		case Intrinsic::llrint: Opcode = ISD::LLRINT; break;
case Intrinsic::nearbyint: Opcode = ISD::FNEARBYINT; break;		case Intrinsic::nearbyint: Opcode = ISD::FNEARBYINT; break;
case Intrinsic::round: Opcode = ISD::FROUND; break;		case Intrinsic::round: Opcode = ISD::FROUND; break;
case Intrinsic::minnum: Opcode = ISD::FMINNUM; break;		case Intrinsic::minnum: Opcode = ISD::FMINNUM; break;
case Intrinsic::maxnum: Opcode = ISD::FMAXNUM; break;		case Intrinsic::maxnum: Opcode = ISD::FMAXNUM; break;
case Intrinsic::umul_with_overflow: Opcode = ISD::UMULO; break;		case Intrinsic::umul_with_overflow: Opcode = ISD::UMULO; break;
case Intrinsic::smul_with_overflow: Opcode = ISD::SMULO; break;		case Intrinsic::smul_with_overflow: Opcode = ISD::SMULO; break;
}		}
}		}
▲ Show 20 Lines • Show All 614 Lines • Show Last 20 Lines

llvm/test/CodeGen/PowerPC/llrint-ctr-crash.ll

This file was added.

				; RUN: llc -verify-machineinstrs -O1 < %s \| FileCheck %s

				; These tests ensure that LLVM will not incorrectly optimize a loop when using
				; lrint and llrint intrinsics. The test cases are largely adapted from
				; ctrloop-*.ll test cases.

				; CHECK: lrint_f32_ctr_crash
				; CHECK: bl lrintf
				; CHECK-NOT: bdnz
				; a < b
				define void @lrint_f32_ctr_crash(i32* nocapture %p, i32 %a, i32 %b) nounwind {
				entry:
				%cmp3 = icmp slt i32 42, %b
				br i1 %cmp3, label %for.body.lr.ph, label %for.end

				for.body.lr.ph: ; preds = %entry
				br label %for.body

				for.body: ; preds = %for.body.lr.ph, %for.body
				%i.04 = phi i32 [ 42, %for.body.lr.ph ], [ %inc, %for.body ]
				%arrayidx = getelementptr inbounds i32, i32* %p, i32 %i.04
				%0 = load i32, i32* %arrayidx, align 1
				%lrint = tail call i32 @llvm.lrint.f32(float 42.0)
				store i32 %lrint, i32* %arrayidx, align 1
				%inc = add nsw i32 %i.04, 1
				%cmp = icmp slt i32 %inc, %b
				br i1 %cmp, label %for.body, label %for.end

				for.end: ; preds = %for.body, %entry
				ret void
				}

				; CHECK: lrint_f64_ctr_crash
				; CHECK: bl lrint
				; CHECK-NOT: bdnz
				; a < b
				define void @lrint_f64_ctr_crash(i32* nocapture %p, i32 %a, i32 %b) nounwind {
				entry:
				%cmp3 = icmp slt i32 42, %b
				br i1 %cmp3, label %for.body.lr.ph, label %for.end

				for.body.lr.ph: ; preds = %entry
				br label %for.body

				for.body: ; preds = %for.body.lr.ph, %for.body
				%i.04 = phi i32 [ 42, %for.body.lr.ph ], [ %inc, %for.body ]
				%arrayidx = getelementptr inbounds i32, i32* %p, i32 %i.04
				%0 = load i32, i32* %arrayidx, align 1
				%lrint = tail call i32 @llvm.lrint.f64(double 42.0)
				store i32 %lrint, i32* %arrayidx, align 1
				%inc = add nsw i32 %i.04, 1
				%cmp = icmp slt i32 %inc, %b
				br i1 %cmp, label %for.body, label %for.end

				for.end: ; preds = %for.body, %entry
				ret void
				}

				; CHECK: lrint_f128_ctr_crash
				; CHECK: bl lrintl
				; CHECK-NOT: bdnz
				; a < b
				define void @lrint_f128_ctr_crash(i32* nocapture %p, i32 %a, i32 %b) nounwind {
				entry:
				%cmp3 = icmp slt i32 42, %b
				br i1 %cmp3, label %for.body.lr.ph, label %for.end

				for.body.lr.ph: ; preds = %entry
				br label %for.body

				for.body: ; preds = %for.body.lr.ph, %for.body
				%i.04 = phi i32 [ 42, %for.body.lr.ph ], [ %inc, %for.body ]
				%arrayidx = getelementptr inbounds i32, i32* %p, i32 %i.04
				%0 = load i32, i32* %arrayidx, align 1
				%n = sitofp i8 42 to fp128
				%lrint = tail call i32 @llvm.lrint.f128(fp128 %n)
				store i32 %lrint, i32* %arrayidx, align 1
				%inc = add nsw i32 %i.04, 1
				%cmp = icmp slt i32 %inc, %b
				br i1 %cmp, label %for.body, label %for.end

				for.end: ; preds = %for.body, %entry
				ret void
				}

				; CHECK: llrint_f32_ctr_crash
				; CHECK: bl llrintf
				; CHECK-NOT: bdnz
				; a < b
				define void @llrint_f32_ctr_crash(i64* nocapture %p, i32 %a, i32 %b) nounwind {
				entry:
				%cmp3 = icmp slt i32 42, %b
				br i1 %cmp3, label %for.body.lr.ph, label %for.end

				for.body.lr.ph: ; preds = %entry
				br label %for.body

				for.body: ; preds = %for.body.lr.ph, %for.body
				%i.04 = phi i32 [ 42, %for.body.lr.ph ], [ %inc, %for.body ]
				%arrayidx = getelementptr inbounds i64, i64* %p, i32 %i.04
				%0 = load i64, i64* %arrayidx, align 1
				%llrint = tail call i64 @llvm.llrint.f32(float 42.0)
				store i64 %llrint, i64* %arrayidx, align 1
				%inc = add nsw i32 %i.04, 1
				%cmp = icmp slt i32 %inc, %b
				br i1 %cmp, label %for.body, label %for.end

				for.end: ; preds = %for.body, %entry
				ret void
				}

				; CHECK: llrint_f64_ctr_crash
				; CHECK: bl llrint
				; CHECK-NOT: bdnz
				; a < b
				define void @llrint_f64_ctr_crash(i64* nocapture %p, i32 %a, i32 %b) nounwind {
				entry:
				%cmp3 = icmp slt i32 42, %b
				br i1 %cmp3, label %for.body.lr.ph, label %for.end

				for.body.lr.ph: ; preds = %entry
				br label %for.body

				for.body: ; preds = %for.body.lr.ph, %for.body
				%i.04 = phi i32 [ 42, %for.body.lr.ph ], [ %inc, %for.body ]
				%arrayidx = getelementptr inbounds i64, i64* %p, i32 %i.04
				%0 = load i64, i64* %arrayidx, align 1
				%llrint = tail call i64 @llvm.llrint.f64(double 42.0)
				store i64 %llrint, i64* %arrayidx, align 1
				%inc = add nsw i32 %i.04, 1
				%cmp = icmp slt i32 %inc, %b
				br i1 %cmp, label %for.body, label %for.end

				for.end: ; preds = %for.body, %entry
				ret void
				}

				; CHECK: llrint_f128_ctr_crash
				; CHECK: bl llrintl
				; CHECK-NOT: bdnz
				; a < b
				define void @llrint_f128_ctr_crash(i64* nocapture %p, i32 %a, i32 %b) nounwind {
				entry:
				%cmp3 = icmp slt i32 42, %b
				br i1 %cmp3, label %for.body.lr.ph, label %for.end

				for.body.lr.ph: ; preds = %entry
				br label %for.body

				for.body: ; preds = %for.body.lr.ph, %for.body
				%i.04 = phi i32 [ 42, %for.body.lr.ph ], [ %inc, %for.body ]
				%arrayidx = getelementptr inbounds i64, i64* %p, i32 %i.04
				%0 = load i64, i64* %arrayidx, align 1
				%n = sitofp i8 42 to fp128
				%llrint = tail call i64 @llvm.llrint.f128(fp128 %n)
				store i64 %llrint, i64* %arrayidx, align 1
				%inc = add nsw i32 %i.04, 1
				%cmp = icmp slt i32 %inc, %b
				br i1 %cmp, label %for.body, label %for.end

				for.end: ; preds = %for.body, %entry
				ret void
				}

				declare i32 @llvm.lrint.f32(float) nounwind readnone
				declare i32 @llvm.lrint.f64(double) nounwind readnone
				declare i32 @llvm.lrint.f128(fp128) nounwind readnone
				declare i64 @llvm.llrint.f32(float) nounwind readnone
				declare i64 @llvm.llrint.f64(double) nounwind readnone
				declare i64 @llvm.llrint.f128(fp128) nounwind readnone