Page MenuHomePhabricator

[mir] Fix confusing MIR when MMO's value is nullptr but offset is non-zero
ClosedPublic

Authored by dsanders on Mar 2 2021, 3:03 PM.

Details

Summary

:: (store 1 + 4, addrspace 1)
->
:: (store 1 into undef + 4, addrspace 1)

An offset without a base isn't terribly useful but it's convenient to update
the offset without checking the value. For example, when breaking apart
stores into smaller units

Diff Detail

Unit TestsFailed

TimeTest
40 msx64 windows > LLVM.ExecutionEngine/JITLink/AArch64::MachO_arm64_ehframe.test
Script: -- : 'RUN: at line 1'; c:\ws\w1\llvm-project\premerge-checks\build\bin\llvm-jitlink.exe -noexec C:\ws\w1\llvm-project\premerge-checks\llvm\test\ExecutionEngine\JITLink\AArch64/Inputs/MachO_arm64_ehframe.o
60 msx64 windows > LLVM.ExecutionEngine/JITLink/AArch64::MachO_arm64_relocations.s
Script: -- : 'RUN: at line 1'; rm -rf C:\ws\w1\llvm-project\premerge-checks\build\test\ExecutionEngine\JITLink\AArch64\Output\MachO_arm64_relocations.s.tmp && mkdir -p C:\ws\w1\llvm-project\premerge-checks\build\test\ExecutionEngine\JITLink\AArch64\Output\MachO_arm64_relocations.s.tmp
70 msx64 windows > LLVM.ExecutionEngine/JITLink/X86::ELF_skip_debug_sections.s
Script: -- : 'RUN: at line 2'; c:\ws\w1\llvm-project\premerge-checks\build\bin\llvm-mc.exe -triple=x86_64-pc-linux-gnu -filetype=obj -o C:\ws\w1\llvm-project\premerge-checks\build\test\ExecutionEngine\JITLink\X86\Output\ELF_skip_debug_sections.s.tmp C:\ws\w1\llvm-project\premerge-checks\llvm\test\ExecutionEngine\JITLink\X86\ELF_skip_debug_sections.s
120 msx64 windows > LLVM.ExecutionEngine/JITLink/X86::ELF_weak_definitions.s
Script: -- : 'RUN: at line 1'; rm -rf C:\ws\w1\llvm-project\premerge-checks\build\test\ExecutionEngine\JITLink\X86\Output\ELF_weak_definitions.s.tmp && mkdir -p C:\ws\w1\llvm-project\premerge-checks\build\test\ExecutionEngine\JITLink\X86\Output\ELF_weak_definitions.s.tmp
50 msx64 windows > LLVM.ExecutionEngine/JITLink/X86::ELF_x86-64_common.s
Script: -- : 'RUN: at line 1'; rm -rf C:\ws\w1\llvm-project\premerge-checks\build\test\ExecutionEngine\JITLink\X86\Output\ELF_x86-64_common.s.tmp && mkdir -p C:\ws\w1\llvm-project\premerge-checks\build\test\ExecutionEngine\JITLink\X86\Output\ELF_x86-64_common.s.tmp
View Full Test Results (19 Failed)

Event Timeline

dsanders created this revision.Mar 2 2021, 3:03 PM
dsanders requested review of this revision.Mar 2 2021, 3:03 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 2 2021, 3:03 PM

Note: I went with undef in this patch because the keyword already exists but it occurred to me while posting that that could also be confusing. The location is defined, we just don't know what it is. We might want to consider something more direct like 'unknown'. Any thoughts on the spelling to use?

qcolombet accepted this revision.Mar 4 2021, 9:38 AM

Hi Daniel,

We might want to consider something more direct like 'unknown'. Any thoughts on the spelling to use?

I agree that "unknown" would be easier to understand but I think we can fix that in a follow-up patch if that proves to be non-trivial. The current printing is super confusing anyway so this is an improvement IMO :).

Cheers,
-Quentin

This revision is now accepted and ready to land.Mar 4 2021, 9:38 AM
This revision was landed with ongoing or failed builds.Mar 4 2021, 10:35 AM
This revision was automatically updated to reflect the committed changes.