Please use GitHub pull requests for new patches. Avoid migrating existing patches. Phabricator shutdown timeline
Changeset View
Changeset View
Standalone View
Standalone View
lib/Target/ARM/ARMSubtarget.cpp
| Show First 20 Lines • Show All 70 Lines • ▼ Show 20 Lines | |||||
| /// so that we can use initializer lists for subtarget initialization. | /// so that we can use initializer lists for subtarget initialization. | ||||
| ARMSubtarget &ARMSubtarget::initializeSubtargetDependencies(StringRef CPU, | ARMSubtarget &ARMSubtarget::initializeSubtargetDependencies(StringRef CPU, | ||||
| StringRef FS) { | StringRef FS) { | ||||
| initializeEnvironment(); | initializeEnvironment(); | ||||
| initSubtargetFeatures(CPU, FS); | initSubtargetFeatures(CPU, FS); | ||||
| return *this; | return *this; | ||||
| } | } | ||||
| /// EnableExecuteOnly - Enables the generation of execute-only code on supported | |||||
| /// targets | |||||
| static cl::opt<bool> | |||||
| EnableExecuteOnly("arm-execute-only"); | |||||
| ARMFrameLowering *ARMSubtarget::initializeFrameLowering(StringRef CPU, | ARMFrameLowering *ARMSubtarget::initializeFrameLowering(StringRef CPU, | ||||
| StringRef FS) { | StringRef FS) { | ||||
| ARMSubtarget &STI = initializeSubtargetDependencies(CPU, FS); | ARMSubtarget &STI = initializeSubtargetDependencies(CPU, FS); | ||||
| if (STI.isThumb1Only()) | if (STI.isThumb1Only()) | ||||
| return (ARMFrameLowering *)new Thumb1FrameLowering(STI); | return (ARMFrameLowering *)new Thumb1FrameLowering(STI); | ||||
| return new ARMFrameLowering(STI); | return new ARMFrameLowering(STI); | ||||
| } | } | ||||
| ARMSubtarget::ARMSubtarget(const Triple &TT, const std::string &CPU, | ARMSubtarget::ARMSubtarget(const Triple &TT, const std::string &CPU, | ||||
| const std::string &FS, | const std::string &FS, | ||||
| const ARMBaseTargetMachine &TM, bool IsLittle) | const ARMBaseTargetMachine &TM, bool IsLittle) | ||||
| : ARMGenSubtargetInfo(TT, CPU, FS), UseMulOps(UseFusedMulOps), | : ARMGenSubtargetInfo(TT, CPU, FS), UseMulOps(UseFusedMulOps), | ||||
| CPUString(CPU), IsLittle(IsLittle), TargetTriple(TT), Options(TM.Options), | CPUString(CPU), IsLittle(IsLittle), TargetTriple(TT), Options(TM.Options), | ||||
| TM(TM), FrameLowering(initializeFrameLowering(CPU, FS)), | TM(TM), GenExecuteOnly(EnableExecuteOnly), | ||||
| FrameLowering(initializeFrameLowering(CPU, FS)), | |||||
| // At this point initializeSubtargetDependencies has been called so | // At this point initializeSubtargetDependencies has been called so | ||||
| // we can query directly. | // we can query directly. | ||||
| InstrInfo(isThumb1Only() | InstrInfo(isThumb1Only() | ||||
| ? (ARMBaseInstrInfo *)new Thumb1InstrInfo(*this) | ? (ARMBaseInstrInfo *)new Thumb1InstrInfo(*this) | ||||
| : !isThumb() | : !isThumb() | ||||
| ? (ARMBaseInstrInfo *)new ARMInstrInfo(*this) | ? (ARMBaseInstrInfo *)new ARMInstrInfo(*this) | ||||
| : (ARMBaseInstrInfo *)new Thumb2InstrInfo(*this)), | : (ARMBaseInstrInfo *)new Thumb2InstrInfo(*this)), | ||||
| TLInfo(TM, *this), GISel() {} | TLInfo(TM, *this), GISel() {} | ||||
| ▲ Show 20 Lines • Show All 62 Lines • ▼ Show 20 Lines | else | ||||
| ArchFS = FS; | ArchFS = FS; | ||||
| } | } | ||||
| ParseSubtargetFeatures(CPUString, ArchFS); | ParseSubtargetFeatures(CPUString, ArchFS); | ||||
| // FIXME: This used enable V6T2 support implicitly for Thumb2 mode. | // FIXME: This used enable V6T2 support implicitly for Thumb2 mode. | ||||
| // Assert this for now to make the change obvious. | // Assert this for now to make the change obvious. | ||||
| assert(hasV6T2Ops() || !hasThumb2()); | assert(hasV6T2Ops() || !hasThumb2()); | ||||
| // Execute only support requires movt support | |||||
| if (genExecuteOnly()) | |||||
rengolin: I'm worried about the silent behaviour here.
If movt is disabled by some other part or user… | |||||
Not Done ReplyInline ActionsYes, this should be an error. I've changed this to report an error if NoMovt is enabled or the target doesn't support MOVT. I'll also add a diagnostic to clang for this. prakhar: Yes, this should be an error. I've changed this to report an error if NoMovt is enabled or the… | |||||
Not Done ReplyInline ActionsThe error should be detected earlier and a proper diagnostic issued if at all possible. Using report_fatal_error() for user-facing diagnostics is an option of last resort and an indicator of a layering failure elsewhere. grosbach: The error should be detected earlier and a proper diagnostic issued if at all possible. Using… | |||||
Not Done ReplyInline ActionsThere is an error in Clang (D27450) that detects this. Is there a better way to do this in LLVM itself? rengolin: There is an error in Clang (D27450) that detects this. Is there a better way to do this in LLVM… | |||||
Not Done ReplyInline ActionsIf there's a legitimate user-facing way to get here and this be the first time we can diagnose the error, we can use the emitError() method on the LLVMContext to complain about it. If, OTOH, getting here in that state indicates a bug earlier in the pipeline (probably true?), then perhaps this could just be an assert(). grosbach: If there's a legitimate user-facing way to get here and this be the first time we can diagnose… | |||||
Not Done ReplyInline ActionsThere shouldn't be a user-facing way to get here, though it may be possible possible with the right combination of flags on llc/llvm-mc. A quick assert would be of the same value as the one above, I'm ok with that, too. rengolin: There shouldn't be a user-facing way to get here, though it may be possible possible with the… | |||||
Not Done ReplyInline ActionsSGTM. I don't think it's too horrible if llc/llvm-mc can trigger asserts. Not ideal, but those aren't (supposed to be) user-facing tools at all. grosbach: SGTM. I don't think it's too horrible if llc/llvm-mc can trigger asserts. Not ideal, but those… | |||||
Not Done ReplyInline ActionsIt's possible to reach this point with llc/llvm-mc by specifying +execute-only and +no-movt for -target-feature. As I'll be adding clang diagnostics for -mno-movt being enabled alongside -mexecute-only, I'll revert this back to an assert. prakhar: It's possible to reach this point with llc/llvm-mc by specifying +execute-only and +no-movt for… | |||||
| assert(hasV8MBaselineOps() && !NoMovt && "Cannot generate execute-only code for this target"); | |||||
This assert is wrong. It should be something like: assert(hasV8MBaselineOps() && "Execute-only supported by ARMv8M only"); And it should only be an assert if there are no combination of user flags that can get you here. If there are, this should be an error. rengolin: This assert is wrong. It should be something like:
assert(hasV8MBaselineOps() && "Execute… | |||||
| // Keep a pointer to static instruction cost data for the specified CPU. | // Keep a pointer to static instruction cost data for the specified CPU. | ||||
| SchedModel = getSchedModelForCPU(CPUString); | SchedModel = getSchedModelForCPU(CPUString); | ||||
| // Initialize scheduling itinerary for the specified CPU. | // Initialize scheduling itinerary for the specified CPU. | ||||
| InstrItins = getInstrItineraryForCPU(CPUString); | InstrItins = getInstrItineraryForCPU(CPUString); | ||||
| // FIXME: this is invalid for WindowsCE | // FIXME: this is invalid for WindowsCE | ||||
| if (isTargetWindows()) | if (isTargetWindows()) | ||||
| ▲ Show 20 Lines • Show All 168 Lines • ▼ Show 20 Lines | bool ARMSubtarget::useStride4VFPs(const MachineFunction &MF) const { | ||||
| return isTargetWatchABI() || (isSwift() && !MF.getFunction()->optForMinSize()); | return isTargetWatchABI() || (isSwift() && !MF.getFunction()->optForMinSize()); | ||||
| } | } | ||||
| bool ARMSubtarget::useMovt(const MachineFunction &MF) const { | bool ARMSubtarget::useMovt(const MachineFunction &MF) const { | ||||
| // NOTE Windows on ARM needs to use mov.w/mov.t pairs to materialise 32-bit | // NOTE Windows on ARM needs to use mov.w/mov.t pairs to materialise 32-bit | ||||
| // immediates as it is inherently position independent, and may be out of | // immediates as it is inherently position independent, and may be out of | ||||
| // range otherwise. | // range otherwise. | ||||
| return !NoMovt && hasV8MBaselineOps() && | return !NoMovt && hasV8MBaselineOps() && | ||||
| (isTargetWindows() || !MF.getFunction()->optForMinSize()); | (isTargetWindows() || !MF.getFunction()->optForMinSize() || genExecuteOnly()); | ||||
| } | } | ||||
| bool ARMSubtarget::useFastISel() const { | bool ARMSubtarget::useFastISel() const { | ||||
| // Enable fast-isel for any target, for testing only. | // Enable fast-isel for any target, for testing only. | ||||
| if (ForceFastISel) | if (ForceFastISel) | ||||
| return true; | return true; | ||||
| // Limit fast-isel to the targets that are or have been tested. | // Limit fast-isel to the targets that are or have been tested. | ||||
| if (!hasV6Ops()) | if (!hasV6Ops()) | ||||
| return false; | return false; | ||||
| // Thumb2 support on iOS; ARM support on iOS, Linux and NaCl. | // Thumb2 support on iOS; ARM support on iOS, Linux and NaCl. | ||||
| return TM.Options.EnableFastISel && | return TM.Options.EnableFastISel && | ||||
| ((isTargetMachO() && !isThumb1Only()) || | ((isTargetMachO() && !isThumb1Only()) || | ||||
| (isTargetLinux() && !isThumb()) || (isTargetNaCl() && !isThumb())); | (isTargetLinux() && !isThumb()) || (isTargetNaCl() && !isThumb())); | ||||
| } | } | ||||
I'm worried about the silent behaviour here.
If movt is disabled by some other part or user flag, than this should be an error, not a silent change.