This is an archive of the discontinued LLVM Phabricator instance.

[clang][Arm] Fix the default floating point ABI for 'armv7-pc-win32-macho'
ClosedPublic

Authored by sarahpurohit on Aug 11 2021, 6:11 PM.

Download Raw Diff

Details

Reviewers

ahatanak
sarahpurohit

Commits

rGee620b1743bc: [clang][Arm] Fix the default floating point ABI for

Summary

It is incorrect to select the hardware floating point ABI on Mach-O platforms
using the Windows triple if the ABI is "apcs-gnu".

rdar://81810554

Diff Detail

Repository: rG LLVM Github Monorepo

Event Timeline

sarahpurohit created this revision.Aug 11 2021, 6:11 PM

Herald added a subscriber: kristof.beyls. · View Herald TranscriptAug 11 2021, 6:11 PM

sarahpurohit requested review of this revision.Aug 11 2021, 6:11 PM

Herald added a subscriber: cfe-commits. · View Herald TranscriptAug 11 2021, 6:11 PM

sarahpurohit retitled this revision from [clang][Arm to [clang][Arm] Fix the default floating point ABI for 'armv7-pc-win32-macho'.Aug 11 2021, 6:15 PM

sarahpurohit edited the summary of this revision. (Show Details)

sarahpurohit added a reviewer: ahatanak.

Harbormaster completed remote builds in B119170: Diff 365885.Aug 11 2021, 6:38 PM

LGTM

clang/lib/Driver/ToolChains/Arch/ARM.cpp
319	You can omit the braces. https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements

This revision is now accepted and ready to land.Aug 12 2021, 6:31 PM

I do not have commit access. Can someone pull this?

Resolved syntax error with braces. No functional change otherwise.

sarahpurohit marked an inline comment as done.Aug 12 2021, 8:57 PM

Harbormaster completed remote builds in B119385: Diff 366179.Aug 12 2021, 9:39 PM

Closed by commit rGee620b1743bc: [clang][Arm] Fix the default floating point ABI for (authored by sarahpurohit, committed by ahatanak). · Explain WhyAug 12 2021, 9:48 PM

This revision was automatically updated to reflect the committed changes.

ahatanak added a commit: rGee620b1743bc: [clang][Arm] Fix the default floating point ABI for.

Revision Contents

Path

Size

clang/

lib/

Driver/

ToolChains/

Arch/

ARM.cpp

4 lines

test/

Driver/

windows-macho.c

11 lines

Diff 366179

clang/lib/Driver/ToolChains/Arch/ARM.cpp

Context not available.

	// FIXME: this is invalid for WindowsCE	// FIXME: this is invalid for WindowsCE
	case llvm::Triple::Win32:	case llvm::Triple::Win32:
		// It is incorrect to select hard float ABI on MachO platforms if the ABI is
		// "apcs-gnu".
		if (Triple.isOSBinFormatMachO() && !useAAPCSForMachO(Triple))
		ahatanakUnsubmitted Done Reply Inline Actions You can omit the braces. https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements ahatanak: You can omit the braces. https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple…
		return FloatABI::Soft;
	return FloatABI::Hard;	return FloatABI::Hard;

	case llvm::Triple::NetBSD:	case llvm::Triple::NetBSD:
Context not available.

clang/test/Driver/windows-macho.c

This file was added.

				// RUN: %clang -target armv7-pc-win32-macho -msoft-float -### -c %s 2>&1 \
				// RUN: \| FileCheck %s --check-prefix CHECK-SOFTFLOAT
				// CHECK-SOFTFLOAT-NOT: error: unsupported option '-msoft-float' for target 'thumbv7-pc-windows-macho'

				// RUN: %clang -target armv7-pc-win32-macho -mhard-float -### -c %s 2>&1 \
				// RUN: \| FileCheck %s --check-prefix CHECK-HARDFLOAT
				// CHECK-HARDFLOAT: error: unsupported option '-mhard-float' for target 'thumbv7-pc-windows-macho'

				// RUN: %clang -target armv7-pc-win32-macho -### -c %s 2>&1 \
				// RUN: \| FileCheck %s --check-prefix CHECK-DEFAULT-SOFTFLOAT-ABI
				// CHECK-DEFAULT-SOFTFLOAT-ABI: "-mfloat-abi" "soft"