This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Remove include of RISCVRegisterInfo.h from RISCVBaseInfo.h
ClosedPublic

Authored by craig.topper on Oct 28 2020, 10:59 AM.

Details

Summary

RISCVRegisterInfo.h is part of the CodeGen layer. The Utils library is intended to be shared with the MC layer so shouldn't use files from the CodeGen layer.

The register enum names are already available from RISCVMCTargetDesc.h. It appears what was coming from this include was a transitive include of the Register class which I've replaced with MCRegister. Register has a constructor from MCRegister so it should be convertible.

Diff Detail

Event Timeline

craig.topper created this revision.Oct 28 2020, 10:59 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 28 2020, 10:59 AM
craig.topper requested review of this revision.Oct 28 2020, 10:59 AM
This revision is now accepted and ready to land.Oct 29 2020, 6:34 AM
craig.topper closed this revision.Oct 29 2020, 2:58 PM

Forgot to put the Differential Revision tag. This was commited as rG22c38376345670c1883963e5e1cccd597a15b3a5

@craig.topper This is causing a MSVC build failure in RISCVTargetLowering::getRegForInlineAsmConstraint

if (Subtarget.hasStdExtF() || Subtarget.hasStdExtD()) {
  std::pair<Register, Register> FReg =
      StringSwitch<std::pair<Register, Register>>(Constraint.lower())
          .Cases("{f0}", "{ft0}", {RISCV::F0_F, RISCV::F0_D})
          .Cases("{f1}", "{ft1}", {RISCV::F1_F, RISCV::F1_D})

with this build failure:

E:\llvm\llvm-project\llvm\include\llvm/Support/type_traits.h(180): error C2338: inconsistent behavior between llvm:: and std:: implementation of is_trivially_copyable
E:\llvm\llvm-project\llvm\include\llvm/ADT/Optional.h(216): note: see reference to class template instantiation 'llvm::is_trivially_copyable<T>' being compiled
        with
        [
            T=std::pair<llvm::Register,llvm::Register>
        ]
E:\llvm\llvm-project\llvm\include\llvm/ADT/StringSwitch.h(48): note: see reference to class template instantiation 'llvm::Optional<T>' being compiled
        with
        [
            T=std::pair<llvm::Register,llvm::Register>
        ]
E:\llvm\llvm-project\llvm\lib\Target\RISCV\RISCVISelLowering.cpp(2725): note: see reference to class template instantiation 'llvm::StringSwitch<std::pair<llvm::Register,llvm::Register>,T>' being compiled
        with
        [
            T=std::pair<llvm::Register,llvm::Register>
        ]
E:\llvm\llvm-project\llvm\include\llvm/ADT/Optional.h(216): error C2976: 'llvm::optional_detail::OptionalStorage': too few template arguments
E:\llvm\llvm-project\llvm\include\llvm/ADT/Optional.h(36): note: see declaration of 'llvm::optional_detail::OptionalStorage'

It looks like this only happens on my EXPENSIVE_CHECKS MSVC (vs2019) build

D86126 might solve this

craig.topper added a comment.EditedOct 30 2020, 10:37 AM

@craig.topper This is causing a MSVC build failure in RISCVTargetLowering::getRegForInlineAsmConstraint

if (Subtarget.hasStdExtF() || Subtarget.hasStdExtD()) {
  std::pair<Register, Register> FReg =
      StringSwitch<std::pair<Register, Register>>(Constraint.lower())
          .Cases("{f0}", "{ft0}", {RISCV::F0_F, RISCV::F0_D})
          .Cases("{f1}", "{ft1}", {RISCV::F1_F, RISCV::F1_D})

with this build failure:

E:\llvm\llvm-project\llvm\include\llvm/Support/type_traits.h(180): error C2338: inconsistent behavior between llvm:: and std:: implementation of is_trivially_copyable
E:\llvm\llvm-project\llvm\include\llvm/ADT/Optional.h(216): note: see reference to class template instantiation 'llvm::is_trivially_copyable<T>' being compiled
        with
        [
            T=std::pair<llvm::Register,llvm::Register>
        ]
E:\llvm\llvm-project\llvm\include\llvm/ADT/StringSwitch.h(48): note: see reference to class template instantiation 'llvm::Optional<T>' being compiled
        with
        [
            T=std::pair<llvm::Register,llvm::Register>
        ]
E:\llvm\llvm-project\llvm\lib\Target\RISCV\RISCVISelLowering.cpp(2725): note: see reference to class template instantiation 'llvm::StringSwitch<std::pair<llvm::Register,llvm::Register>,T>' being compiled
        with
        [
            T=std::pair<llvm::Register,llvm::Register>
        ]
E:\llvm\llvm-project\llvm\include\llvm/ADT/Optional.h(216): error C2976: 'llvm::optional_detail::OptionalStorage': too few template arguments
E:\llvm\llvm-project\llvm\include\llvm/ADT/Optional.h(36): note: see declaration of 'llvm::optional_detail::OptionalStorage'

I'm having trouble finding the connect to my patch. RISCVISelLowering.cpp has its own include of RISCVRegisterInfo.h. And it doesn't appear to call either getSCSPReg() or getBPReg().

Does using unsigned instead of Register in getRegForInlineAsmConstraint fix it? The Register from the std::pair is turned back into an unsigned for the return value of the function anyway.