Diff Detail
Event Timeline
I can't speak to processors prior to power3, but you'd be hard-pressed to find one of those dawn-of-time systems operating today anyway. Mostly this looks fine. However, I am unclear why you have different values for "generic" and "ppc" which appear to be otherwise identical. (But I don't know what "generic" is used for, either.) If "generic" means minimum possible instruction set, I would think you would leave FeatureSYNC out for it.
I had originally left HasSYNC out of generic, but added it in because it broke a bunch of tests (which did not specify anything about the processor and yet were expecting syncs). Should I instead modify all of these tests to use a given processor ? If so which one ?
As I stated, I do not know what the intent of generic is. But "ppc" is also a "generic" 32-bit processor and has the same description otherwise. Why do you leave FeatureSYNC off of it? It seems to my vague understanding that they should be the same.
I did not know that ppc is also supposed to be a generic processor. I can
either add HasSYNC to it, or remove it from generic (and change the broken
tests). Which would you prefer ?
601 and others had sync.
It's somewhat bizarre to me to define new patterns and features to handle something that encodes to the same bit patterns.
I always assumed "generic" was for the purposes of generating code that ran on the maximum set of hardware and was scheduled in a way to do as well as possible without being specific. More or less, the kind of code that would be used by Mac developers.
I don't have a preference because I don't know the history and intent of these processor values. I am just raising the question because the inconsistency needs to be understood. Perhaps hfinkel has ideas.
- Original Message -----
From: "Alex Rosenberg" <alexr@leftfield.org>
To: morisset@google.com, jfb@chromium.org, hfinkel@anl.gov, wschmidt@linux.vnet.ibm.com
Cc: alexr@leftfield.org, llvm-commits@cs.uiuc.edu
Sent: Thursday, September 11, 2014 6:03:46 PM
Subject: Re: [PATCH] [Power] Add a new feature flag for support of the sync instruction601 and others had sync.
As far as I can tell, the 440 and friends did not, and that's really what we're dealing with here. There was some divergence in earlier Book E processors, where they supported only msync (which is indeed equivalent to sync 0), but not any of the other sync forms. More recent Book E processors, such as the A2, support "sync" generally.
As far as I can tell, GNU binutils will accept "sync 0" when targeting the PPC 440 (or any PPC core for that matter), so we don't *need* to print the instruction as "msync" (assuming GNU binutils is what we care about). We could just make "msync" and assembler alias for "sync 0" and be done with it (while this does not match the letter of the ISA, it will certainly work well enough in practice). I'm okay with that solution.
-Hal
It's somewhat bizarre to me to define new patterns and features to
handle something that encodes to the same bit patterns.I always assumed "generic" was for the purposes of generating code
that ran on the maximum set of hardware and was scheduled in a way
to do as well as possible without being specific. More or less, the
kind of code that would be used by Mac developers.
- Original Message -----
From: "Bill Schmidt" <wschmidt@linux.vnet.ibm.com>
To: morisset@google.com, jfb@chromium.org, hfinkel@anl.gov, wschmidt@linux.vnet.ibm.com
Cc: alexr@leftfield.org, llvm-commits@cs.uiuc.edu
Sent: Thursday, September 11, 2014 6:18:13 PM
Subject: Re: [PATCH] [Power] Add a new feature flag for support of the sync instructionI don't have a preference because I don't know the history and intent
of these processor values. I am just raising the question because
the inconsistency needs to be understood. Perhaps hfinkel has
ideas.
I think we can take "ppc" to represent a commodity ppc32 core, and I'd not restrict it to a common subset with early Book E cores. Printing "sync 0" on "ppc", regardless of what is done for Book E generally, is fine.
-Hal
- Original Message -----
From: "Robin Morisset" <morisset@google.com>
To: morisset@google.com, jfb@chromium.org, wschmidt@linux.vnet.ibm.com, hfinkel@anl.gov
Cc: alexr@leftfield.org, llvm-commits@cs.uiuc.edu
Sent: Thursday, October 2, 2014 5:55:16 PM
Subject: Re: [PATCH] [Power] Add a new feature flag for support of the sync instructionDuplicate with commit r218923 by Hal FInkel.
Ack, sorry. I had completely forgotten that you had also started this.
-Hal