This is an archive of the discontinued LLVM Phabricator instance.

[Doc] Refactor descriptions of `min-legal-vector-width`
Needs ReviewPublic

Authored by pengfei on Dec 10 2022, 10:19 PM.

Details

Summary

Based on comments from @arsenm.

Diff Detail

Event Timeline

pengfei created this revision.Dec 10 2022, 10:19 PM
Herald added a project: Restricted Project. · View Herald TranscriptDec 10 2022, 10:19 PM
Herald added a subscriber: StephenFan. · View Herald Transcript
pengfei requested review of this revision.Dec 10 2022, 10:19 PM
Herald added a project: Restricted Project. · View Herald TranscriptDec 10 2022, 10:19 PM
arsenm requested changes to this revision.Dec 13 2022, 3:51 PM
arsenm added inline comments.
llvm/docs/LangRef.rst
2236–2237 ↗(On Diff #481896)

The first sentence isn't grammatically correct. Also should not start out with the word legal

2239 ↗(On Diff #481896)

Still not stating what legal means in this context

2240 ↗(On Diff #481896)

Grammar?

2241 ↗(On Diff #481896)

return types?

Also what is the behavior for aggregates? Aggregates containing vectors? Nested aggregates with vectors?

2243 ↗(On Diff #481896)

Returnings isn't a word; "return values"?

2244–2245 ↗(On Diff #481896)

We can't spec attributes in terms of what clang's behavior is today

This revision now requires changes to proceed.Dec 13 2022, 3:51 PM

I want to remove the description from LangRef directly since we decided to only support this attribute in X86. WDYT?

llvm/docs/LangRef.rst
2241 ↗(On Diff #481896)

Good question. I think we have considered such cases. See https://github.com/llvm/llvm-project/blob/main/clang/lib/CodeGen/CGCall.cpp#L4729-L4740

pengfei updated this revision to Diff 484805.Dec 22 2022, 5:35 AM

Move doc to comments.

Herald added a project: Restricted Project. · View Herald TranscriptDec 22 2022, 5:35 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
pengfei added inline comments.Dec 22 2022, 5:42 AM
clang/lib/CodeGen/CodeGenFunction.cpp
502–504

I still think the sentence is ill-formed. But I have tried my best. Welcome for suggestions.