On Thu, 10 Aug 2023 20:19:02 PDT (-0700), jeffreya...@gmail.com wrote:
On 8/10/23 20:30, Palmer Dabbelt wrote:
Sorry for being lost here, but I'm not sure where TYPE_UNKNOWN comes
from. There's not a whole lot of instances in the code, and they all
seem to be doing something very special. Is it just something we didn't
do a '(set_attr "type" ...)' on?
Yup. TYPE_UNKNOWN means we don't have a type associated with the insn.
As I've mentioned before this isn't a major problem if there's one or
two here and there. But if most are TYPE_UNKNOWN, the the scheduler is
going to do highly unnatural things.
OK, that seems like the way to go. I still think it's likely we'll need
to split up these types more, but that's something we can only deal with
when there's HW that behaves oddly.
In that case it seems reasonable to have a dev-mode early failure: we've
got some odd types now (like just the broad "bitmanip" one), but those
can be split later. At least having some classification seems like the
way to go, it's just an internal interface so we can make it better later.
That said, it also smells like this is something that should be more
generic than backend code?
No, it's really a target issue. And what I was suggesting is that we
get to the point where we can enable the currently #if 0'd assert so
that if we introduce insns without an associated type, we get a nice
early warning. I wasn't up for tackling that this week ;-)
I was thinking of some sort of "TARGET_ALLOWS_UNKNOWN_INSNS" hook, but
poking around the uses that might not be meaningfully simpler than just
rejecting these in the backend -- certainly simpler if we're just
worried about RISC-V ;)
This seems pretty mechinacial: just scrub through our MDs to check for
any un-typed insns, then add the assert and fix the failures. You're
more than welcome to have at it, but LMK if you want me to try and find
some time for someone to do it -- certainly seems like a good way for
someone new to dig in a bit.
> The other thing if this code probably wants to handle GHOST type >
instructions. While GHOST is used for instructions which generate no
> code, it might seem they should return "more" as those INSNs take
no > resources. But GHOST is actually used for things like the
blockage insn > which should end a cycle from an issue standpoint.
So the right > handling of a GHOST is something like this:
> > if (type == TYPE_GHOST)
> return 0;
Lost again, here, there's almost no references to TYPE_GHOST (aside from
a MIPS-ism that looks to have ended up in Loongarch).
Search for "ghost" in riscv.md ;-)
Thanks, the "return 0" makes sense.
Jeff
