Patch by David Chisnall
I've discovered some important background while reviewing this patch. It seems that isMips64() predicates have been used incorrectly as a synonym for various things that aren't MIPS64 but are closely related to it. In particular some of it's uses ought to be checking for the use of the N32 or N64 ABI's rather than tests for MIPS64. I'm happy to sort this out. I mention it because it is likely to affect your patch.
I have mixed opinions about the IsGP64bit feature bit. I see your point that many of the isMips64() conditions are tests of the register size. On the other hand, the newer 64-bit cores are supersets of the older ones so we could also change the test for isMips3(). On balance, I think I'm in favour of a specific IsGP64bit feature bit that is set along with the architecture bit.
Lastly, it needs a basic test case to test that -arch=mips4 exists and functions. An empty assembly file that uses llvm-readobj to test the ELF e_flags would do the trick.
We should rename this function to isGP64bit(), however we probably also need to keep isMips64() to guard instructions code paths that use instructions that were added in MIPS64.
There seem to be two pre-existing mistakes involving this function which need fixing (either of us can do this, I hope to be able to get to it next week).
The second is that both uses of this function in matchCPURegisterName() are really testing for either N32 or N64 rather than for MIPS64.
|656–657 ↗||(On Diff #7132)|
The predicate here should be isMips4() which should test for FeatureMips4
I generally agree with the use of IsGP64bit but we may need to hang on to HasMips64 as well.
This is a pre-existing issue, but this looks wrong. I'm told that in GCC, functions are 4-byte aligned for in O32, N32, and N64 ABI's. I'll look into this.
I need to find out if this is the right predicate. I'm not sure if the use of MO_GOT_OFST is related to 64-bit targets in general, >=MIPS64 specifically, or ABI's.
What ever the outcome of that investigation, the parameter in the declaration and body will need to be renamed to match the new meaning.
I've put the first patch onto phabricator so that I can comment on it (I don't have a github account). I hope that's ok with you.
Most of the issues are related to pre-existing mistakes in the use of MIPS64 predicates which I'm happy to sort out. Other than that, there's a few minor things to correct.
I'm going to look into this branch today and tomorrow. Are you ok with me directly committing things that I'm happy with?
From: Dr D. Chisnall [mailto:email@example.com] On Behalf Of David
Sent: 20 March 2014 14:58
Cc: Daniel Sanders
Subject: Re: [PATCH] Initial MIPS IV support. Separate out the is-MIPS64-ISA
checks from is-64-bit-MIPS tests.