- User Since
- Jul 27 2020, 8:22 AM (25 w, 1 d)
A little bump to get approval.
Sat, Jan 16
I incorporated the various suggestions.
Fri, Jan 15
A lot of the changes are in a function named 'AddSubClass', which is a misnomer. It's adding a parent class, not a subclass. Does anyone mind if I rename the function as part of this revision, or should I do it in a separate revision?
Thu, Jan 14
Tue, Jan 12
Mon, Jan 11
I will look at this issue tomorrow and should have a patch soon.
That is plenty of information, thanks. The problem is that the paste operator (#) is not willing to convert a bit to a string. I have no idea why and I will fix this if there is no good reason to prevent it. Meanwhile, you can use:
I fixed the broken test. Thanks for catching this, Craig.
What am I waiting for? Here is a test of this new message.
I incorporated Craig's suggestions.
Could you post a simple file that reproduces the problem? Without the context of BaseOpcodeSTSrcKind and all the identifiers being pasted, I cannot tell what's going on. It certainly isn't necessary to declare a field 'field' just to set it to a concatenated value.
I will write tests for these errors.
It is deprecated, yes. I'm still investigating whether any TableGen files actually rely on its behavior, which is to allow nonconcrete field values.
Fri, Jan 8
Thu, Jan 7
Wed, Jan 6
The message is now required in the assert statement.
Fixed indentation requested by @craig.topper.
Tue, Jan 5
@lattner Did you mean to accept this revision?
I understand requiring the message, but I don't understand the parentheses. It makes the 'assert' statement syntactically incompatible with the 'if' and 'foreach' statements. Also, it makes it look like assert is a function.
Bumping this for a "looks good."
Bumping this for a final "looks good."
Sun, Jan 3
@RKSimon has a good question that I will leave to others to debate.
Sat, Jan 2
I was thinking of using a bit in the other flags member, Flags, as opposed to TSFlags. Then there would be nothing sneaky going on in the union itself.
@jrtc27 Will some sort of union of a uint64_t and a pointer work? One of the Flags bits could specify which one it is.
If it isn't possible to encode the information in TSFlags, another possibility is to add a pointer to the MCInstrDesc class that points to some kind of auxiliary class/struct. It would be unfortunate to increase the size of MCInstrDesc, though. I have been playing with the idea of changing the three pointer members to indexes in order to save memory, but I'm not yet convinced it's worth the effort. There are about 67,000 instances across the targets.
Fri, Jan 1
Wed, Dec 30
I've abandoned this revision until I spend some time studying it.
I made Craig's suggested changes. I also added one more test to the test file.
Tue, Dec 29
I improved the summary.
Mon, Dec 28
Sat, Dec 26
Let's actually update the patch this time.
interleaveIntList() needs to use convertInitializerTo(), not getCastTo. I'm also using dyn_cast_or_null, although it should never be null.
Fri, Dec 25
Wed, Dec 23
Okay, that sounds like a good plan. I'll let this revision float for now. Shall I email you about profiling?
Ah yes, good point. I was concerned about time, not memory.
I'm not sure where to go from here, then. Do you think I should start over, by eliminating the caching and then profiling?
I'm not yet clever enough to do profiling. It just seemed like building large record vectors multiple times was a waste of time. Perhaps I embarked on this change without sufficient evidence of its necessity.
Changed the loop in CodeEmitterGen.cpp to a range-based loop.
Tue, Dec 22
I will change to a rang-based for loop tomorrow.
There are about 15 call sites in these compilation units. I figured that was a good number to include with the ArrayRef change itself. I had to change OpInterfacesGen.cpp because it would not compile.
I've gone through about eight backends for the first revision. I've used 'auto' when no copy is needed, and added a comment when a copy is needed. I found two places that copy the returned vector but probably don't need to, so I added TODOs there for later consideration.
The modification occurs on line 65:
This update changes getAllDerivedDefinitions so it returns an ArrayRef.
I am about to update this revision to return an ArrayRef. But there is one compilation unit so far that actually modifies the returned array. Stay tuned for my update.
All righty then. Yes, that appears to do the trick.
I'm afraid not. However, a search of the code base reveals that std::numeric_limits<int64_t>::max() is used all over the place, so I think we're okay.
Mon, Dec 21
I suddenly thought that the change to ArrayRef would require me to change every call to getAllDerivedDefinitions in one revision. So I changed the function and tried compiling. I got no complaints. What mechanism is it that allows an ArrayRef to be returned to a variable declared [const] std::vector<Record *> [&] without complaint?
Then this does sound like a good idea.
Is ArrayRef better in some stylistic sense? I'm happy to switch to it.
Change SIZE_MAX to std::numeric_limits<int64_t>::max() to fix a build failure.
What is the correct way to copy the first !substr commit and then amend it with the fix for this build problem?
I will do that in the future.
Well now, that's new to me. Thanks!
I'm sorry, what is a -ve value?
I reverted this revision.
Works like a charm when I run the TableGen tests on my machine. Hmm.
Yes, that is what needs to be done: The assertions have to be stored somehow and checked later.
Dec 20 2020
Change early clobber so its presence is all that matters.
Dec 19 2020
I've been doing it for documentation changes, commenting, and certain other relatively trivial changes, That was suggested by @lattner, not decided unilaterally.
Let's try this again.
Cleaned out the commented-out code.
Dec 18 2020
Incorporated @jansvoboda11 comments.