For more details about these instructions, please refer to the latest ISE document: https://www.intel.com/content/www/us/en/develop/download/intel-architecture-instruction-set-extensions-programming-reference.html
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
clang/include/clang/Driver/Options.td | ||
---|---|---|
4650 | I notice in line 4655 the option name is "prfch", do we need to follow the naming convention? | |
llvm/lib/Target/X86/X86InstrInfo.td | ||
3007 | Could you add comments to explain the what the constant (1, 3, 0) means? I guess it is the same to the arguments that llvm.prefetch defines. |
- Address review comments;
- Optimize prefetchit0/1 to prefetcht0/1 for non-rip address;
- Add semacheck for prefetch write to instruction cache;
- Fix a bug that set rw = 1 for prefetchit0/1;
I think the affected ARM and SystemZ tests are not valid before. Could @t.p.northover and @uweigand help to have a look?
clang/include/clang/Driver/Options.td | ||
---|---|---|
4650 | I don't know the reason why we used prfchw before, but we are now using the option the same as feature name so that user can easily guess the right option. | |
llvm/lib/Target/X86/X86InstrInfo.td | ||
3007 | Added short introduction. Full description is in https://llvm.org/docs/LangRef.html#llvm-prefetch-intrinsic |
This seems to be a semantic change. The Language Reference does not spell out that a write prefetch on the instruction cache is prohibited. In fact, I read it to explicitly state that every flavor of the prefetch intrinsic that doesn't match anything supported on the target architecture should simply be a no-op. (I could imagine that on certain architectures, you might even have such a prefetch, e.g. to handle self-modifying code more efficiently.)
Sure, it is possible. But at least for now, there's no real target requires it. Checked with grep -rwn 'llvm.prefetch.*i32 0\s*)' llvm/test/CodeGen/.
The reason I do it is it's ambiguity to backend when lower it to a target that has "write data", "read data" and "read instruction" prefetches. It's clear if we just support the semantic of "read instruction" and lower it to "read data" when available.
I think we can modify Language Reference for it.
But that's just within the LLVM sources. My point was that -up to now- this was part of the public LLVM IR spec, so there could by other LLVM users out there creating this IR, which would now suddenly break. Not sure what the rules are for breaking IR changes, but that would presumably need some wider discussion.
There are too many things in this patch. Please up MC layer changes from intrinsic/builtin changes. The RIP optimization probably also be separate.
clang/lib/Headers/prfchiintrin.h | ||
---|---|---|
17 | That should have said "It looks ODD that this indented differently..." |
clang/lib/Headers/prfchiintrin.h | ||
---|---|---|
17 | We use this indention in most existing headers. I guess it's intended to align with parameters' description. We follow this rule in the ISAs. |
I notice in line 4655 the option name is "prfch", do we need to follow the naming convention?