[NFC] Fix semantic discrepancy for MVT::LAST_VALUETYPE
ClosedPublic
Actions

Authored by gchatelet on May 27 2021, 7:27 AM.

Details

Reviewers

courbet

Commits

rG1da2c7d25c09: [NFC] Fix semantic discrepancy for MVT::LAST_VALUETYPE

Summary

This patch fixes the discrepancy between all MVT::LAST_XXX enum values.

Usually LAST_XXX values are inclusive (see LAST_INTEGER_VALUETYPE or LAST_FP_VALUETYPE) but LAST_VALUETYPE behaves differently and points past the last value (See https://github.com/llvm/llvm-project/blob/438cf5577e720f84d493a272c5a1cbaf6ce19e51/llvm/include/llvm/Support/MachineValueType.h#L32)

The discrepancy is easy to spot in the _valuetypes() function at the bottom of the file
https://github.com/llvm/llvm-project/blob/438cf5577e720f84d493a272c5a1cbaf6ce19e51/llvm/include/llvm/Support/MachineValueType.h#L1370

This patch makes LAST_VALUETYPE point to the last value (as per definition) and introduces a special enum to be used as the size of a table to hold all enums up to - and including - LAST_VALUETYPE.

The patch is a bit verbose because of formatting issues the Linter was complaining about.

Diff Detail

Repository: rG LLVM Github Monorepo

Unit TestsFailed
View All

	Time	Test
	270 ms	x64 windows > LLVM.DebugInfo/X86::basic-block-sections-debug-loc-const-value-1.ll Script: -- : 'RUN: at line 1'; c:\ws\w3\llvm-project\premerge-checks\build\bin\llc.exe C:\ws\w3\llvm-project\premerge-checks\llvm\test\DebugInfo\X86\basic-block-sections-debug-loc-const-value-1.ll --dwarf-version=4 --basic-block-sections=none -filetype=obj -o C:\ws\w3\llvm-project\premerge-checks\build\test\DebugInfo\X86\Output\basic-block-sections-debug-loc-const-value-1.ll.tmp
	270 ms	x64 windows > LLVM.DebugInfo/X86::basic-block-sections-debug-loc-const-value-2.ll Script: -- : 'RUN: at line 1'; c:\ws\w3\llvm-project\premerge-checks\build\bin\llc.exe C:\ws\w3\llvm-project\premerge-checks\llvm\test\DebugInfo\X86\basic-block-sections-debug-loc-const-value-2.ll --dwarf-version=4 --basic-block-sections=none -filetype=obj -o C:\ws\w3\llvm-project\premerge-checks\build\test\DebugInfo\X86\Output\basic-block-sections-debug-loc-const-value-2.ll.tmp
	230 ms	x64 windows > LLVM.DebugInfo/X86::basic-block-sections-debug-loc.ll Script: -- : 'RUN: at line 5'; c:\ws\w3\llvm-project\premerge-checks\build\bin\llc.exe C:\ws\w3\llvm-project\premerge-checks\llvm\test\DebugInfo\X86\basic-block-sections-debug-loc.ll --dwarf-version=4 --basic-block-sections=none -filetype=obj -o C:\ws\w3\llvm-project\premerge-checks\build\test\DebugInfo\X86\Output\basic-block-sections-debug-loc.ll.tmp
	230 ms	x64 windows > LLVM.DebugInfo/X86::basic-block-sections-debug-loclist-4.ll Script: -- : 'RUN: at line 1'; c:\ws\w3\llvm-project\premerge-checks\build\bin\llc.exe C:\ws\w3\llvm-project\premerge-checks\llvm\test\DebugInfo\X86\basic-block-sections-debug-loclist-4.ll --dwarf-version=4 --basic-block-sections=none -filetype=obj -o C:\ws\w3\llvm-project\premerge-checks\build\test\DebugInfo\X86\Output\basic-block-sections-debug-loclist-4.ll.tmp
	220 ms	x64 windows > LLVM.DebugInfo/X86::basic-block-sections-debug-loclist-5.ll Script: -- : 'RUN: at line 1'; c:\ws\w3\llvm-project\premerge-checks\build\bin\llc.exe C:\ws\w3\llvm-project\premerge-checks\llvm\test\DebugInfo\X86\basic-block-sections-debug-loclist-5.ll --dwarf-version=4 --basic-block-sections=none -filetype=obj -o C:\ws\w3\llvm-project\premerge-checks\build\test\DebugInfo\X86\Output\basic-block-sections-debug-loclist-5.ll.tmp

Event Timeline

gchatelet created this revision.May 27 2021, 7:27 AM

Herald added subscribers: dexonsmith, hiraditya, dschuff. · View Herald TranscriptMay 27 2021, 7:27 AM

gchatelet requested review of this revision.May 27 2021, 7:27 AM

Herald added a project: Restricted Project. · View Herald TranscriptMay 27 2021, 7:27 AM

Herald added subscribers: llvm-commits, aheejin. · View Herald Transcript

gchatelet added inline comments.May 27 2021, 7:30 AM

llvm/include/llvm/Support/MachineValueType.h
39	Unfortunately the Linter forced me to reformat this whole section.
262–263	The gist of this patch is the modification of these two lines.

Harbormaster completed remote builds in B106509: Diff 348260.May 27 2021, 8:28 AM

rebase

gchatelet edited the summary of this revision. (Show Details)Jun 7 2021, 2:11 AM

Harbormaster completed remote builds in B107926: Diff 350199.Jun 7 2021, 2:42 AM

courbet accepted this revision.Jun 7 2021, 2:46 AM

courbet added inline comments.

llvm/include/llvm/Support/MachineValueType.h
1373	Why not `VALUETYPE_SIZE` ?

This revision is now accepted and ready to land.Jun 7 2021, 2:46 AM

gchatelet marked an inline comment as done.Jun 7 2021, 3:02 AM

gchatelet added inline comments.

llvm/include/llvm/Support/MachineValueType.h
1373	For consistency with the functions below. I have a follow up patch that will simplify all these functions so it's simpler if they look the same.

This revision was landed with ongoing or failed builds.Jun 7 2021, 3:04 AM

Closed by commit rG1da2c7d25c09: [NFC] Fix semantic discrepancy for MVT::LAST_VALUETYPE (authored by gchatelet). · Explain Why

This revision was automatically updated to reflect the committed changes.

gchatelet marked an inline comment as done.

gchatelet added a commit: rG1da2c7d25c09: [NFC] Fix semantic discrepancy for MVT::LAST_VALUETYPE.

craig.topper added a subscriber: craig.topper.Jun 7 2021, 3:24 PM

craig.topper added inline comments.

llvm/include/llvm/Support/MachineValueType.h
39	I really would have rather put clang-format off/on directives around this. We've been manually formatting this for a very long time because it makes it much easier to see and increment the numbers when new types are added.

gchatelet added inline comments.Jun 8 2021, 12:31 AM