Page MenuHomePhabricator

[RISCV][Clang] Implement support for zmmul-experimental
Needs ReviewPublic

Authored by ksyx on May 28 2021, 6:18 AM.

Details

Summary

This patch implements recently merged extension Zmmul, a subextension of M (Integer Multiplication and Division) consisting only multiplication part of it.

Diff Detail

Event Timeline

ksyx created this revision.May 28 2021, 6:18 AM
ksyx requested review of this revision.May 28 2021, 6:18 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptMay 28 2021, 6:18 AM

Do we need to add this to RISCVTargetStreamer::emitTargetAttributes?

Do we need to add this to RISCVTargetStreamer::emitTargetAttributes?

Yes, that, but also RISCVAsmParser::parseDirectiveAttribute and ELFObjectFileBase::getRISCVFeatures, though it seems that last one has been a bit neglected.

clang/lib/Basic/Targets/RISCV.cpp
294–295

These are supposed to be (approximately) sorted in terms of their canonical ordering. I don't know where Zmmul is proposed to go relative to the other Zfoo's, but it belongs down somewhere in there, I guess alphabetical if we don't know any better, not up here with the single-letter extensions.

llvm/lib/Target/RISCV/RISCVSubtarget.h
38

Ditto

107

Ditto

Do we need to add this to RISCVTargetStreamer::emitTargetAttributes?

Yes, that, but also RISCVAsmParser::parseDirectiveAttribute and ELFObjectFileBase::getRISCVFeatures, though it seems that last one has been a bit neglected.

(we really need to cut down on how many places parse/produce arch strings...)

ksyx updated this revision to Diff 348608.May 28 2021, 6:18 PM
ksyx marked an inline comment as done.
  • Move down zmmul lines to fit the ordering
  • Add Zmmul into parseDirectiveAttribute
ksyx added a comment.May 28 2021, 6:19 PM

I have took a look into ELFObjectFile.cpp but I am not sure what work I need to do there since it seems other Z* extensions are not being handled there.

llvm/lib/Target/RISCV/RISCVSubtarget.h
38

The pattern here seems to be that Z* extensions is following its parent their parent extensions, should we follow it? (See B and V)

ksyx added inline comments.May 28 2021, 6:23 PM
llvm/lib/Target/RISCV/RISCVSubtarget.h
38

But for Zfh it is not following HasStdExtF 🤔

Jim added a comment.Jun 9 2021, 11:33 PM

It should add arch string tests such as in clang/test/Driver/riscv-arch.c.

ksyx updated this revision to Diff 351083.Jun 10 2021, 1:04 AM

add: zmmul arch string test