Page MenuHomePhabricator

Add accessors for MCSubtargetInfo CPU and Feature tables
AcceptedPublic

Authored by jrmuizel on Wed, Dec 23, 6:10 PM.

Details

Summary

This is needed for -C target-cpu=help and -C target-feature=help in rustc

Diff Detail

Unit TestsFailed

TimeTest
15,410 msx64 debian > LLVM.Bindings/Go::go.test
Script: -- : 'RUN: at line 1'; /mnt/disks/ssd0/agent/llvm-project/build/bin/llvm-go go=/usr/bin/go test llvm.org/llvm/bindings/go/llvm
70 msx64 windows > LLVM.CodeGen/XCore::threads.ll
Script: -- : 'RUN: at line 1'; c:\ws\w1\llvm-project\premerge-checks\build\bin\llc.exe -march=xcore < C:\ws\w1\llvm-project\premerge-checks\llvm\test\CodeGen\XCore\threads.ll | c:\ws\w1\llvm-project\premerge-checks\build\bin\filecheck.exe C:\ws\w1\llvm-project\premerge-checks\llvm\test\CodeGen\XCore\threads.ll

Event Timeline

jrmuizel created this revision.Wed, Dec 23, 6:10 PM
jrmuizel requested review of this revision.Wed, Dec 23, 6:10 PM
Herald added a project: Restricted Project. · View Herald TranscriptWed, Dec 23, 6:10 PM
nikic accepted this revision.Sun, Jan 17, 1:51 AM

LGTM. I'm not particularly familiar with this code, but don't see an issue with exposing this information. Currently MCSubtargetInfo directly incorporates the logic for printing a help text to the error stream, which is very dubious design. It should be possible to obtain the same information for printing a help text manually.

This revision is now accepted and ready to land.Sun, Jan 17, 1:51 AM

@nikic can you land this?

fhahn added a subscriber: fhahn.Sun, Jan 17, 6:28 AM
fhahn added inline comments.
llvm/include/llvm/MC/MCSubtargetInfo.h
229

Can you add doc-comments for the new functions?

jrmuizel updated this revision to Diff 317217.Sun, Jan 17, 6:56 AM

Add comments

I’m not against this, but do want to mention the help text is incorrect for x86-64. It lists CPUs that don’t support 64-bit. Using one of those CPUs will trigger a fatal error. A better list is fillValidCPUList from X86TargetParser.cpp in lib/Support. It takes a mode argument. That’s the interface clang uses.

I filed https://github.com/rust-lang/rust/issues/81148 about improving the list the Rust uses.

jrmuizel updated this revision to Diff 317250.Sun, Jan 17, 5:04 PM

Update the doc comments to be less wrong.

I’m not against this, but do want to mention the help text is incorrect for x86-64. It lists CPUs that don’t support 64-bit. Using one of those CPUs will trigger a fatal error. A better list is fillValidCPUList from X86TargetParser.cpp in lib/Support. It takes a mode argument. That’s the interface clang uses.

The fillValidCPUArchList() interface looks pretty annoying in that it's non-virtual. One needs to call different helper functions for each target.