This is an archive of the discontinued LLVM Phabricator instance.

Apply llvm-prefer-register-over-unsigned from clang-tidy to LLVM
ClosedPublic

Authored by dsanders on Aug 8 2019, 11:09 AM.

Details

Summary

This clang-tidy check is looking for unsigned integer variables whose initializer
starts with an implicit cast from llvm::Register and changes the type of the
variable to llvm::Register (dropping the llvm:: where possible).

Partial reverts in:
X86FrameLowering.cpp - Some functions return unsigned and arguably should be MCRegister
X86FixupLEAs.cpp - Some functions return unsigned and arguably should be MCRegister
X86FrameLowering.cpp - Some functions return unsigned and arguably should be MCRegister
HexagonBitSimplify.cpp - Function takes BitTracker::RegisterRef which appears to be unsigned&
MachineVerifier.cpp - Ambiguous operator==() given MCRegister and const Register
PPCFastISel.cpp - No Register::operator-=()
PeepholeOptimizer.cpp - TargetInstrInfo::optimizeLoadInstr() takes an unsigned&
MachineTraceMetrics.cpp - MachineTraceMetrics lacks a suitable constructor

Manual fixups in:
ARMFastISel.cpp - ARMEmitLoad() now takes a Register& instead of unsigned&
HexagonSplitDouble.cpp - Ternary operator was ambiguous between unsigned/Register
HexagonConstExtenders.cpp - Has a local class named Register, used llvm::Register instead of Register.
PPCFastISel.cpp - PPCEmitLoad() now takes a Register& instead of unsigned&

Depends on D65919

Diff Detail

Event Timeline

dsanders created this revision.Aug 8 2019, 11:09 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 8 2019, 11:09 AM

LGTM for the AArch64 changes.

WebAssembly change LGTM.

lenary added a subscriber: lenary.Aug 12 2019, 1:59 AM

The RISC-V backend changes look good to me. There are more changes that could be done, but we can do most of them by hand, or they're in target-independent interfaces that have not yet changed.

WebAssembly changes: r368627 (6aa77e740bf)
AArch64 changes: r368628 (b12454126d1)
RISC-V changes: r368629 (51880816fbc)

Thanks Amara, Heejin, and Sam. I'll rebase this patch once the monorepo syncs

dsanders updated this revision to Diff 214727.Aug 12 2019, 3:44 PM

Rebase now that AArch64, RISC-V, and WebAssembly portions have landed

dsanders edited the summary of this revision. (Show Details)Aug 12 2019, 3:45 PM
arsenm accepted this revision.Aug 13 2019, 9:44 AM

LGTM

This revision is now accepted and ready to land.Aug 13 2019, 9:44 AM
RKSimon added subscribers: craig.topper, RKSimon.

@craig.topper You might want to look at the x86 changes (and the manual reversions that @dsanders mentions)

X86 changes look good to me. I'll see about addressing some of the reverts after this goes in.

X86 changes look good to me. I'll see about addressing some of the reverts after this goes in.

Thanks, they should be fairly simple. The only reason they're partial reverts instead of manual fixups was because it wasn't completely trivial and localized and I didn't want to complicate the patch as it was already large

This revision was automatically updated to reflect the committed changes.
asbirlea removed a subscriber: asbirlea.Aug 15 2019, 1:27 PM