Skip to content

Add MSP430 toolchain#5

Closed
chbessonova wants to merge 1 commit intoaccess-softek:masterfrom
chbessonova:msp430-toolchain
Closed

Add MSP430 toolchain#5
chbessonova wants to merge 1 commit intoaccess-softek:masterfrom
chbessonova:msp430-toolchain

Conversation

@chbessonova
Copy link
Contributor

@chbessonova chbessonova commented Oct 18, 2018

Introduces MSP430 toolchain, -mmcu, -mhwmult options support.
Please, take a look at.
I also would appreciate for some help with
[MSP430] Add msp430 devices list to MSP430.td
178f732 (see a comment in commit message).

@mskvortsov
Copy link
Contributor

mskvortsov commented Oct 18, 2018

Regarding device identification, I think it's better not to pass a device name to the backend.
clang -cc1 -help gives the following info:

  -target-abi <value>     Target a particular ABI type
  -target-cpu <value>     Target a specific cpu type
  -target-feature <value> Target specific attributes

As far as I understand, -target-cpu can be one of msp430, msp430x or maybe msp430xv2.
And -target-feature is meant to be one of +hwmult, +hwmultf5, ...

A full device name is needed only for linker to load a specific linker script.

@asl
Copy link
Contributor

asl commented Oct 18, 2018

+1 to @mskvortsov

@chbessonova
Copy link
Contributor Author

chbessonova commented Oct 18, 2018 •
edited
Loading

A full device name is needed only for linker to load a specific linker script.

It is also needed to generate a '__MSP430XXX__' define which allows a user to include the generic header 'msp430.h' instead of a target specific which is a widely used way to have a deal with includes.

So, it's needed to pass the full device name to the compiler using some other ways, or decide to not to support the aforementioned feature.

@asl
Copy link
Contributor

asl commented Oct 18, 2018

Right. We're talking about the backend. Essentially the toolchain should parse the device name and transform it into more fine grained stuff:

  • Builtin defines
  • Linker script filenames
  • Libraries, additional run-time objects, etc.
  • Target features

asl
asl reviewed Oct 18, 2018
View reviewed changes
asl
asl reviewed Oct 18, 2018
View reviewed changes
asl
asl reviewed Oct 18, 2018
View reviewed changes
src/llvm/tools/clang/include/clang/Basic/DiagnosticDriverKinds.td Outdated
"hardware multiplier, but -mhwmult is set to %1.">,
InGroup<InvalidCommandLineArgument>;
def warn_drv_msp430_hwmult_no_device : Warning<"No MCU device specified, but "
"'-mhwmult' is set to 'auto', assuming no hardware multiply. Use -mmcu to "
Copy link
Contributor

@asl asl Oct 18, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

multiply => multiplier?

Copy link
Contributor Author

@chbessonova chbessonova Oct 19, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds okay to me. I'm even thinking about change to 'multiply' in the other warnings cause 'supports' 'multiplier' looks really weird.

asl
asl reviewed Oct 18, 2018
View reviewed changes
@chbessonova
Copy link
Contributor Author

chbessonova commented Oct 18, 2018

Right. We're talking about the backend. Essentially the toolchain should parse the device name and transform it into more fine grained stuff:

Yes, I understand the idea.
Let me give more details.

The driver takes -mmcu option with a particular device name and uses it to:

  • translate to cc1 option '-target-cpu %device_name%' (this is a problem)
  • construct correct linker command line /choose linker script name, libraries, etc/ (it's okay).

First is needed to generate device specific defines in frontend. For backend this option is almost unused. But it affects both.

If I keep all as is and just remove 'def : Proc<"msp430c111", []>;' defines from backend, I get an error "'msp430xxx' is not a recognized processor for this target (ignoring processor)".
Could I resolve the error without defining all these devices in backend? I haven't found a way.

Another option is to translate it to some other (maybe new) cc1 option which will be handled in frontend, but doesn't affect backend (it doesn't seems as a good option to me).

@chbessonova
Copy link
Contributor Author

chbessonova commented Oct 18, 2018

Possible solution for the problem above (after discussing with Michael):

  • do not translate -mmcu option to -target-cpu, which allow to remove useless defines from backend.
  • move target-specific defines generation from frontend to driver (and pass it to the compiler like '-D__MSP430XXX__'

It allows to simplify msp430 support in frontend, remove useless code from backend, but likely there will be no diagnostic for unsupported mcus (all mcus names will be accepted).

@asl
Copy link
Contributor

asl commented Oct 18, 2018

Well, we still could check stuff in driver and warn for unknown CPUs :) Otherwise - LGTM.

@asl asl mentioned this pull request Oct 19, 2018
Open
asl
asl reviewed Oct 19, 2018
View reviewed changes
@chbessonova chbessonova force-pushed the msp430-toolchain branch 2 times, most recently from 888a590 to b54dde3 Compare October 23, 2018 16:38
asl pushed a commit that referenced this pull request Nov 26, 2018
@alexfh
Fix uninitialized variable.
b81719b 
Flags variable was not initialized and later used (both isMBBSafeToOutlineFrom
implementations assume it's initialized), which breaks
test/CodeGen/AArch64/machine-outliner.mir. under memory sanitizer:
MemorySanitizer: use-of-uninitialized-value
    #0  in llvm::AArch64InstrInfo::getOutliningType(llvm::MachineInstrBundleIterator<llvm::MachineInstr, false>&, unsigned int) const llvm/lib/Target/AArch64/AArch64InstrInfo.cpp:5494:9
    #1  in (anonymous namespace)::InstructionMapper::convertToUnsignedVec(llvm::MachineBasicBlock&, llvm::TargetInstrInfo const&) llvm/lib/CodeGen/MachineOutliner.cpp:772:19
    #2  in (anonymous namespace)::MachineOutliner::populateMapper((anonymous namespace)::InstructionMapper&, llvm::Module&, llvm::MachineModuleInfo&) llvm/lib/CodeGen/MachineOutliner.cpp:1543:14
    #3  in (anonymous namespace)::MachineOutliner::runOnModule(llvm::Module&) llvm/lib/CodeGen/MachineOutliner.cpp:1645:3
    #4  in (anonymous namespace)::MPPassManager::runOnModule(llvm::Module&) llvm/lib/IR/LegacyPassManager.cpp:1744:27
    #5  in llvm::legacy::PassManagerImpl::run(llvm::Module&) llvm/lib/IR/LegacyPassManager.cpp:1857:44
    #6  in compileModule(char**, llvm::LLVMContext&) llvm/tools/llc/llc.cpp:597:8

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@346761 91177308-0d34-0410-b5e6-96231b3b80d8
chbessonova
chbessonova commented Nov 29, 2018
View reviewed changes
@chbessonova chbessonova mentioned this pull request Dec 20, 2018
Open
asl
asl reviewed Jan 9, 2019
View reviewed changes
asl
asl reviewed Jan 9, 2019
View reviewed changes
asl
asl reviewed Jan 9, 2019
View reviewed changes
src/llvm/tools/clang/lib/Driver/ToolChains/MSP430.cpp
if (!MCUArg && !HWMultArg)
return;

StringRef HWMult = HWMultArg ? HWMultArg->getValue() : "auto";
Copy link
Contributor

@asl asl Jan 9, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Вот этот код как-то вынести в отдельную функцию? Он аналогичен коду выше.

asl
asl reviewed Jan 9, 2019
View reviewed changes
asl
asl reviewed Jan 9, 2019
View reviewed changes
asl
asl reviewed Jan 9, 2019
View reviewed changes
asl
asl reviewed Jan 9, 2019
View reviewed changes
@chbessonova chbessonova force-pushed the msp430-toolchain branch 2 times, most recently from 292b7b6 to 645c6da Compare January 10, 2019 18:18
@chbessonova
Copy link
Contributor Author

chbessonova commented Jan 11, 2019

I don't know what is wrong with Travis. The patch doesn't introduce any changes in the backend and currently 'cleanly' rebased to master, but for some reason, Travis checks out and test another PR - #57.

@dmikushin dmikushin mentioned this pull request Jan 24, 2019
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@asl asl asl left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@chbessonova @mskvortsov @asl