Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[arm64] Constant rematerialization fix for loads clobbering a constant register #479

Closed
wants to merge 2 commits into from

Conversation

Projects
None yet
1 participant
@siddhesh
Copy link

siddhesh commented Feb 22, 2019

During rematerialization, the register allocator is unable to see when the write to a destination register from a register holding a constant overwrites the register and incorrectly uses it for rematerialization, thus causing a crash.

The first patch in this PR creates a new rset that keeps track of such constant regs and adds a new register allocator entry point ra_alloc_rematsafe() that marks the register as unsafe if it holds a constant and is reused for the destination. The arm64 backend uses this entry point and the new rset (rematset) to make sure that it does not use an unsafe constant register for rematerialization either during an ra_allock or in emit_kdelta.

siddhesh added some commits Feb 22, 2019

Fix constant rematerialization when the constant is clobbered by dest
PR #438 describes an issue with constant rematerialization where a
rematerializaed constant could use a register it thought was constant
but was in fact clobbered in the same instruction that it was defined
in.

Fix the problem by adding a mask and a safe entry point for the
allocator that marks constant holding registers that are unsafe for
rematerialization due to them being the same as the destination
register for an instruction it is used in.  While this is only
implemented in asm_fusexref, it should ideally be implemented for all
instructions where the destination is likely to clobber one of the
source registers and if one or more of the source registers hold
constants.
Remove redundant emit_check_ofs
Even if the offset is a constant, it is not 32-bit since it failed
that check earlier before it came here.  The code is thus useless and
hence removed.
@siddhesh

This comment has been minimized.

Copy link
Author

siddhesh commented Mar 14, 2019

I'm dropping this in favour of the earlier fix.

@siddhesh siddhesh closed this Mar 14, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.