[PATCH] D21960: [Sparc] Leon errata fixes passes.

[Daniel Cederman via llvm-commits]

dcederman added a comment.

Some more inline comments.

Could you please also revert the committed changes, like @jyknight asked you?


================
Comment at: lib/Target/Sparc/LeonFeatures.td:88-90
@@ +87,5 @@
+def FillDataCache : SubtargetFeature<"filldatacache", "FillDataCache", "true",
+                                     "LEON2 erratum fix: Ensure data cache is "
+                                     "filled so that cache misses do not "
+                                     "happen later in program execution.">;
+
----------------
A cache miss occurs when the data at the memory address accessed is not available in the cache. Filling the data cache with random data that will not be accessed again will not prevent cache misses, it will make them more likely.

Could you reference the document that describes the errata you are trying to provide a fix for? I looked in http://www.atmel.com/Images/doc4409.pdf but could not find a match.

================
Comment at: lib/Target/Sparc/LeonPasses.cpp:757-762
@@ +756,8 @@
+//
+// mov 0, %l0
+// mov 4096, %l1
+// loop1:
+// inc %l0
+// cmp %l0, %l1
+// ble loop1
+
----------------
lero_chris wrote:
> A cache stores recently-accessed data. To fill the cache, we need to access data big enough to fill the L1 cache of the LEON 2 FT CPU. To solve this problem, we use NOP instructions to fill a block of memory with NOP instructions. The LEON 2’s L1 data cache is 16 KB [6] and the size of the NOP instruction is 4 Bytes. Therefore, we need 4096 NOPs to entirely fill the cache. Note this will happen only once at the first call of the runOnMachineFunction() method.
But you are not accessing data? It is just a loop that does nothing. And if I understand the code correctly, you randomly add it to the first function you find?

================
Comment at: lib/Target/Sparc/LeonPasses.cpp:851-859
@@ +850,11 @@
+//****************************************************************************************************************
+// This erratum fix should handle user traps of FPU exceptions and restore the
+// execution address by skipping the trapped FPU instruction.
+// The algorithm:
+// find rett - return from trap
+// insert code before rett to:
+// 1. load the FSR register
+// 2. check if there is an FPU exception
+// 3. branch to old rett if there is no exception
+// 4. rett to a restored exec address
+char RestoreExecAddress::ID = 0;
----------------
lero_chris wrote:
> Do you suggest that this pass should not be part of the compiler then? I can remove it if it fixes nothing.
You should remove it. Automatically changing assembly code in the trap handler is extremely risky.


Repository:
  rL LLVM

https://reviews.llvm.org/D21960