diff mbox

[RFC,v6,11/14] softmmu: Simplify helper_*_st_name, wrap MMIO code

Message ID 1450082498-27109-12-git-send-email-a.rigo@virtualopensystems.com
State New
Headers show

Commit Message

Alvise Rigo Dec. 14, 2015, 8:41 a.m. UTC
Attempting to simplify the helper_*_st_name, wrap the MMIO code into an
inline function.

Suggested-by: Jani Kokkonen <jani.kokkonen@huawei.com>
Suggested-by: Claudio Fontana <claudio.fontana@huawei.com>
Signed-off-by: Alvise Rigo <a.rigo@virtualopensystems.com>
---
 softmmu_template.h | 64 +++++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 44 insertions(+), 20 deletions(-)

Comments

Alex Bennée Jan. 11, 2016, 9:54 a.m. UTC | #1
Alvise Rigo <a.rigo@virtualopensystems.com> writes:

> Attempting to simplify the helper_*_st_name, wrap the MMIO code into an
> inline function.
>
> Suggested-by: Jani Kokkonen <jani.kokkonen@huawei.com>
> Suggested-by: Claudio Fontana <claudio.fontana@huawei.com>
> Signed-off-by: Alvise Rigo <a.rigo@virtualopensystems.com>
> ---
>  softmmu_template.h | 64 +++++++++++++++++++++++++++++++++++++-----------------
>  1 file changed, 44 insertions(+), 20 deletions(-)
>
> diff --git a/softmmu_template.h b/softmmu_template.h
> index 92f92b1..2ebf527 100644
> --- a/softmmu_template.h
> +++ b/softmmu_template.h
> @@ -396,6 +396,26 @@ static inline void glue(helper_le_st_name, _do_unl_access)(CPUArchState *env,
>      }
>  }
>
> +static inline void glue(helper_le_st_name, _do_mmio_access)(CPUArchState *env,
> +                                                            DATA_TYPE val,
> +                                                            target_ulong addr,
> +                                                            TCGMemOpIdx oi,
> +                                                            unsigned mmu_idx,
> +                                                            int index,
> +                                                            uintptr_t retaddr)
> +{
> +    CPUIOTLBEntry *iotlbentry = &env->iotlb[mmu_idx][index];
> +
> +    if ((addr & (DATA_SIZE - 1)) != 0) {
> +        glue(helper_le_st_name, _do_unl_access)(env, val, addr, mmu_idx,
> +                                                oi, retaddr);
> +    }
> +    /* ??? Note that the io helpers always read data in the target
> +       byte ordering.  We should push the LE/BE request down into io.  */
> +    val = TGT_LE(val);
> +    glue(io_write, SUFFIX)(env, iotlbentry, val, addr, retaddr);
> +}
> +

Some comment as previous patches. I think we can have a single function
that is shared between both helpers.

>  void helper_le_st_name(CPUArchState *env, target_ulong addr, DATA_TYPE val,
>                         TCGMemOpIdx oi, uintptr_t retaddr)
>  {
> @@ -458,16 +478,8 @@ void helper_le_st_name(CPUArchState *env, target_ulong addr, DATA_TYPE val,
>
>              return;
>          } else {
> -            if ((addr & (DATA_SIZE - 1)) != 0) {
> -                glue(helper_le_st_name, _do_unl_access)(env, val, addr, mmu_idx,
> -                                                        oi, retaddr);
> -            }
> -            iotlbentry = &env->iotlb[mmu_idx][index];
> -
> -            /* ??? Note that the io helpers always read data in the target
> -               byte ordering.  We should push the LE/BE request down into io.  */
> -            val = TGT_LE(val);
> -            glue(io_write, SUFFIX)(env, iotlbentry, val, addr, retaddr);
> +            glue(helper_le_st_name, _do_mmio_access)(env, val, addr, oi,
> +                                                     mmu_idx, index, retaddr);
>              return;
>          }
>      }
> @@ -523,6 +535,26 @@ static inline void glue(helper_be_st_name, _do_unl_access)(CPUArchState *env,
>      }
>  }
>
> +static inline void glue(helper_be_st_name, _do_mmio_access)(CPUArchState *env,
> +                                                            DATA_TYPE val,
> +                                                            target_ulong addr,
> +                                                            TCGMemOpIdx oi,
> +                                                            unsigned mmu_idx,
> +                                                            int index,
> +                                                            uintptr_t retaddr)
> +{
> +    CPUIOTLBEntry *iotlbentry = &env->iotlb[mmu_idx][index];
> +
> +    if ((addr & (DATA_SIZE - 1)) != 0) {
> +        glue(helper_be_st_name, _do_unl_access)(env, val, addr, mmu_idx,
> +                                                oi, retaddr);
> +    }
> +    /* ??? Note that the io helpers always read data in the target
> +       byte ordering.  We should push the LE/BE request down into io.  */
> +    val = TGT_BE(val);
> +    glue(io_write, SUFFIX)(env, iotlbentry, val, addr, retaddr);
> +}
> +
>  void helper_be_st_name(CPUArchState *env, target_ulong addr, DATA_TYPE val,
>                         TCGMemOpIdx oi, uintptr_t retaddr)
>  {
> @@ -585,16 +617,8 @@ void helper_be_st_name(CPUArchState *env, target_ulong addr, DATA_TYPE val,
>
>              return;
>          } else {
> -            if ((addr & (DATA_SIZE - 1)) != 0) {
> -                glue(helper_be_st_name, _do_unl_access)(env, val, addr, mmu_idx,
> -                                                        oi, retaddr);
> -            }
> -            iotlbentry = &env->iotlb[mmu_idx][index];
> -
> -            /* ??? Note that the io helpers always read data in the target
> -               byte ordering.  We should push the LE/BE request down into io.  */
> -            val = TGT_BE(val);
> -            glue(io_write, SUFFIX)(env, iotlbentry, val, addr, retaddr);
> +            glue(helper_be_st_name, _do_mmio_access)(env, val, addr, oi,
> +                                                     mmu_idx, index, retaddr);
>              return;
>          }
>      }


--
Alex Bennée
Alvise Rigo Jan. 11, 2016, 10:19 a.m. UTC | #2
On Mon, Jan 11, 2016 at 10:54 AM, Alex Bennée <alex.bennee@linaro.org> wrote:
>
> Alvise Rigo <a.rigo@virtualopensystems.com> writes:
>
>> Attempting to simplify the helper_*_st_name, wrap the MMIO code into an
>> inline function.
>>
>> Suggested-by: Jani Kokkonen <jani.kokkonen@huawei.com>
>> Suggested-by: Claudio Fontana <claudio.fontana@huawei.com>
>> Signed-off-by: Alvise Rigo <a.rigo@virtualopensystems.com>
>> ---
>>  softmmu_template.h | 64 +++++++++++++++++++++++++++++++++++++-----------------
>>  1 file changed, 44 insertions(+), 20 deletions(-)
>>
>> diff --git a/softmmu_template.h b/softmmu_template.h
>> index 92f92b1..2ebf527 100644
>> --- a/softmmu_template.h
>> +++ b/softmmu_template.h
>> @@ -396,6 +396,26 @@ static inline void glue(helper_le_st_name, _do_unl_access)(CPUArchState *env,
>>      }
>>  }
>>
>> +static inline void glue(helper_le_st_name, _do_mmio_access)(CPUArchState *env,
>> +                                                            DATA_TYPE val,
>> +                                                            target_ulong addr,
>> +                                                            TCGMemOpIdx oi,
>> +                                                            unsigned mmu_idx,
>> +                                                            int index,
>> +                                                            uintptr_t retaddr)
>> +{
>> +    CPUIOTLBEntry *iotlbentry = &env->iotlb[mmu_idx][index];
>> +
>> +    if ((addr & (DATA_SIZE - 1)) != 0) {
>> +        glue(helper_le_st_name, _do_unl_access)(env, val, addr, mmu_idx,
>> +                                                oi, retaddr);
>> +    }
>> +    /* ??? Note that the io helpers always read data in the target
>> +       byte ordering.  We should push the LE/BE request down into io.  */
>> +    val = TGT_LE(val);
>> +    glue(io_write, SUFFIX)(env, iotlbentry, val, addr, retaddr);
>> +}
>> +
>
> Some comment as previous patches. I think we can have a single function
> that is shared between both helpers.

Of course. If the objdump you got from this version and the version
with single helper is basically the same, then there's no reason to
make two distinct variants.

Thank you,
alvise

>
>>  void helper_le_st_name(CPUArchState *env, target_ulong addr, DATA_TYPE val,
>>                         TCGMemOpIdx oi, uintptr_t retaddr)
>>  {
>> @@ -458,16 +478,8 @@ void helper_le_st_name(CPUArchState *env, target_ulong addr, DATA_TYPE val,
>>
>>              return;
>>          } else {
>> -            if ((addr & (DATA_SIZE - 1)) != 0) {
>> -                glue(helper_le_st_name, _do_unl_access)(env, val, addr, mmu_idx,
>> -                                                        oi, retaddr);
>> -            }
>> -            iotlbentry = &env->iotlb[mmu_idx][index];
>> -
>> -            /* ??? Note that the io helpers always read data in the target
>> -               byte ordering.  We should push the LE/BE request down into io.  */
>> -            val = TGT_LE(val);
>> -            glue(io_write, SUFFIX)(env, iotlbentry, val, addr, retaddr);
>> +            glue(helper_le_st_name, _do_mmio_access)(env, val, addr, oi,
>> +                                                     mmu_idx, index, retaddr);
>>              return;
>>          }
>>      }
>> @@ -523,6 +535,26 @@ static inline void glue(helper_be_st_name, _do_unl_access)(CPUArchState *env,
>>      }
>>  }
>>
>> +static inline void glue(helper_be_st_name, _do_mmio_access)(CPUArchState *env,
>> +                                                            DATA_TYPE val,
>> +                                                            target_ulong addr,
>> +                                                            TCGMemOpIdx oi,
>> +                                                            unsigned mmu_idx,
>> +                                                            int index,
>> +                                                            uintptr_t retaddr)
>> +{
>> +    CPUIOTLBEntry *iotlbentry = &env->iotlb[mmu_idx][index];
>> +
>> +    if ((addr & (DATA_SIZE - 1)) != 0) {
>> +        glue(helper_be_st_name, _do_unl_access)(env, val, addr, mmu_idx,
>> +                                                oi, retaddr);
>> +    }
>> +    /* ??? Note that the io helpers always read data in the target
>> +       byte ordering.  We should push the LE/BE request down into io.  */
>> +    val = TGT_BE(val);
>> +    glue(io_write, SUFFIX)(env, iotlbentry, val, addr, retaddr);
>> +}
>> +
>>  void helper_be_st_name(CPUArchState *env, target_ulong addr, DATA_TYPE val,
>>                         TCGMemOpIdx oi, uintptr_t retaddr)
>>  {
>> @@ -585,16 +617,8 @@ void helper_be_st_name(CPUArchState *env, target_ulong addr, DATA_TYPE val,
>>
>>              return;
>>          } else {
>> -            if ((addr & (DATA_SIZE - 1)) != 0) {
>> -                glue(helper_be_st_name, _do_unl_access)(env, val, addr, mmu_idx,
>> -                                                        oi, retaddr);
>> -            }
>> -            iotlbentry = &env->iotlb[mmu_idx][index];
>> -
>> -            /* ??? Note that the io helpers always read data in the target
>> -               byte ordering.  We should push the LE/BE request down into io.  */
>> -            val = TGT_BE(val);
>> -            glue(io_write, SUFFIX)(env, iotlbentry, val, addr, retaddr);
>> +            glue(helper_be_st_name, _do_mmio_access)(env, val, addr, oi,
>> +                                                     mmu_idx, index, retaddr);
>>              return;
>>          }
>>      }
>
>
> --
> Alex Bennée
diff mbox

Patch

diff --git a/softmmu_template.h b/softmmu_template.h
index 92f92b1..2ebf527 100644
--- a/softmmu_template.h
+++ b/softmmu_template.h
@@ -396,6 +396,26 @@  static inline void glue(helper_le_st_name, _do_unl_access)(CPUArchState *env,
     }
 }
 
+static inline void glue(helper_le_st_name, _do_mmio_access)(CPUArchState *env,
+                                                            DATA_TYPE val,
+                                                            target_ulong addr,
+                                                            TCGMemOpIdx oi,
+                                                            unsigned mmu_idx,
+                                                            int index,
+                                                            uintptr_t retaddr)
+{
+    CPUIOTLBEntry *iotlbentry = &env->iotlb[mmu_idx][index];
+
+    if ((addr & (DATA_SIZE - 1)) != 0) {
+        glue(helper_le_st_name, _do_unl_access)(env, val, addr, mmu_idx,
+                                                oi, retaddr);
+    }
+    /* ??? Note that the io helpers always read data in the target
+       byte ordering.  We should push the LE/BE request down into io.  */
+    val = TGT_LE(val);
+    glue(io_write, SUFFIX)(env, iotlbentry, val, addr, retaddr);
+}
+
 void helper_le_st_name(CPUArchState *env, target_ulong addr, DATA_TYPE val,
                        TCGMemOpIdx oi, uintptr_t retaddr)
 {
@@ -458,16 +478,8 @@  void helper_le_st_name(CPUArchState *env, target_ulong addr, DATA_TYPE val,
 
             return;
         } else {
-            if ((addr & (DATA_SIZE - 1)) != 0) {
-                glue(helper_le_st_name, _do_unl_access)(env, val, addr, mmu_idx,
-                                                        oi, retaddr);
-            }
-            iotlbentry = &env->iotlb[mmu_idx][index];
-
-            /* ??? Note that the io helpers always read data in the target
-               byte ordering.  We should push the LE/BE request down into io.  */
-            val = TGT_LE(val);
-            glue(io_write, SUFFIX)(env, iotlbentry, val, addr, retaddr);
+            glue(helper_le_st_name, _do_mmio_access)(env, val, addr, oi,
+                                                     mmu_idx, index, retaddr);
             return;
         }
     }
@@ -523,6 +535,26 @@  static inline void glue(helper_be_st_name, _do_unl_access)(CPUArchState *env,
     }
 }
 
+static inline void glue(helper_be_st_name, _do_mmio_access)(CPUArchState *env,
+                                                            DATA_TYPE val,
+                                                            target_ulong addr,
+                                                            TCGMemOpIdx oi,
+                                                            unsigned mmu_idx,
+                                                            int index,
+                                                            uintptr_t retaddr)
+{
+    CPUIOTLBEntry *iotlbentry = &env->iotlb[mmu_idx][index];
+
+    if ((addr & (DATA_SIZE - 1)) != 0) {
+        glue(helper_be_st_name, _do_unl_access)(env, val, addr, mmu_idx,
+                                                oi, retaddr);
+    }
+    /* ??? Note that the io helpers always read data in the target
+       byte ordering.  We should push the LE/BE request down into io.  */
+    val = TGT_BE(val);
+    glue(io_write, SUFFIX)(env, iotlbentry, val, addr, retaddr);
+}
+
 void helper_be_st_name(CPUArchState *env, target_ulong addr, DATA_TYPE val,
                        TCGMemOpIdx oi, uintptr_t retaddr)
 {
@@ -585,16 +617,8 @@  void helper_be_st_name(CPUArchState *env, target_ulong addr, DATA_TYPE val,
 
             return;
         } else {
-            if ((addr & (DATA_SIZE - 1)) != 0) {
-                glue(helper_be_st_name, _do_unl_access)(env, val, addr, mmu_idx,
-                                                        oi, retaddr);
-            }
-            iotlbentry = &env->iotlb[mmu_idx][index];
-
-            /* ??? Note that the io helpers always read data in the target
-               byte ordering.  We should push the LE/BE request down into io.  */
-            val = TGT_BE(val);
-            glue(io_write, SUFFIX)(env, iotlbentry, val, addr, retaddr);
+            glue(helper_be_st_name, _do_mmio_access)(env, val, addr, oi,
+                                                     mmu_idx, index, retaddr);
             return;
         }
     }