LLVM Bugzilla is read-only and represents the historical archive of all LLVM issues filled before November 26, 2021. Use github to submit LLVM bugs

Bug 1306 - LiveVariables improperly handles killing aliased registers
Summary: LiveVariables improperly handles killing aliased registers
Status: RESOLVED FIXED
Alias: None
Product: new-bugs
Classification: Unclassified
Component: new bugs (show other bugs)
Version: unspecified
Hardware: All All
: P normal
Assignee: Evan Cheng
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-04-04 14:42 PDT by Christopher Lamb
Modified: 2010-03-06 13:59 PST (History)
4 users (show)

See Also:
Fixed By Commit(s):


Attachments
Patch 1 (22.73 KB, text/plain)
2007-04-19 19:48 PDT, Evan Cheng
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Christopher Lamb 2007-04-04 14:42:45 PDT
Here is the beginning of the BB dump.

entry (0x8503c80, LLVM BB @0x8501af0, ID#0):
Live Ins: %R0 %R1
	%reg1024 = ORI %R0<kill>, 0
	%reg1025 = ORI %R1<kill>, 0

V4R0 is getting killed because handleLiveInRegister() is called on  
all results of getAliasSet() for each of the liveins (this is in  
LiveIntervals::computeIntervals() ).

handleRegisterDef() does a similar thing where calls  
handlePhysicalRegisterDef() on all members of getAliasSet() returned  
for the def, which also triggers this problem.

---

This is a pretty serious bug. LiveVariables::KillsRegister should not kill aliases that are "larger". The 
correct way to fix this is to explicitly list registers that are defined, used, and killed. So your example 
should look like:

entry (0x8503c80, LLVM BB @0x8501af0, ID#0):
Live Ins: %R0 %R1
	%reg1024 = ORI %R0<kill>, 0, %V4R0<imp-use>
	%reg1025 = ORI %R1<kill>, 0, %V4R0<imp-use,kill>

KillsRegister should check for exact match rather than regsOverlap. There are probably other similar 
bugs in LiveVariables.
Comment 1 Evan Cheng 2007-04-18 18:20:37 PDT
This is causing me much pain.

The correct fix, IMO, is to explicit add every single alias register as use /
def operands. Change LiveVariables and LiveIntervalAnalysis to not iterate
through alias sets when processing registers. Also change
LiveVariables::KillsRegister(), RegisterDefIsDead(), etc. to check for exact
register match rather than regsOverlap().

It's unclear though how copy coalescer handles this though. Anyway, it's a
fairly involved set of changes. Anyone has a better suggestion?
Comment 2 Evan Cheng 2007-04-19 01:52:56 PDT
My brute force fix works well on X86. All of the dejagnu tests are passing as
well. I'll do some more testing on other targets.
Comment 3 Evan Cheng 2007-04-19 19:48:33 PDT
Created attachment 791 [details]
Patch 1
Comment 4 Evan Cheng 2007-04-19 19:49:33 PDT
This is the patch. Chris, please see if it fixes the problems you run into. I'm
working on a better fix.
Comment 5 Christopher Lamb 2007-04-22 20:21:34 PDT
The patch cleared up the issue I was seeing.
Comment 6 Chris Lattner 2007-04-22 20:23:24 PDT
Great!  Evan is working on a version that will be better for compile time and handle more cases.  It should 
land in the next week or so.
Comment 8 Christopher Lamb 2007-04-25 19:35:37 PDT
This is working for me.
Comment 9 Christopher Lamb 2007-04-30 19:03:58 PDT
Should this be marked closed as it's working for me, or is it waiting for more testing?
Comment 10 Chris Lattner 2007-04-30 19:05:17 PDT
Yep, evan should close it.  He's unruly though, and doesn't like to close bugs ;-)
Comment 11 Evan Cheng 2007-05-01 21:16:02 PDT
I like to leave bugs open to give the impression that I am busy.