Discussion:
[gem5-dev] Review Request 3446: hsail, gpu-compute: fixes to appease clang++
(too old to reply)
Tony Gutierrez
2016-04-12 17:13:10 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3446/
-----------------------------------------------------------

Review request for Default.


Repository: gem5


Description
-------

Changeset 11443:81eb66b2fc69
---------------------------
hsail,gpu-compute: fixes to appease clang++

fixes to appease clang++. tested on Ubuntu clang version
3.5.0-4ubuntu2~trusty2 (tags/RELEASE_350/final) (based on
LLVM 3.5.0)

the fixes address the following two issues:

1) the exec continuations in gpu_static_inst.hh were marked
as protected when they should be public. here we mark
them as public

2) the Abs instruction uses std::abs() in its execute method.
because Abs is templated, it can also operate on U32 and U64,
types, which cause Abs::execute() to pass uint32_t and uint64_t
types to std::abs() respectively. this triggers a warning
because std::abs() has no effect in this case. to rememdy this
we add template specialization for the execute() method of Abs
when its template paramter is U32 or U64.


Diffs
-----

src/arch/hsail/gen.py b31738224fb0eb259efc25f6d6efab5a962f29d0
src/gpu-compute/gpu_static_inst.hh b31738224fb0eb259efc25f6d6efab5a962f29d0

Diff: http://reviews.gem5.org/r/3446/diff/


Testing
-------


Thanks,

Tony Gutierrez
Tony Gutierrez
2016-04-12 17:20:12 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3446/
-----------------------------------------------------------

(Updated April 12, 2016, 10:20 a.m.)


Review request for Default.


Repository: gem5


Description (updated)
-------

Changeset 11443:3e0817967515
---------------------------
hsail,gpu-compute: fixes to appease clang++

fixes to appease clang++. tested on Ubuntu clang version
3.5.0-4ubuntu2~trusty2 (tags/RELEASE_350/final) (based on
LLVM 3.5.0)

the fixes address the following two issues:

1) the exec continuations in gpu_static_inst.hh were marked
as protected when they should be public. here we mark
them as public

2) the Abs instruction uses std::abs() in its execute method.
because Abs is templated, it can also operate on U32 and U64,
types, which cause Abs::execute() to pass uint32_t and uint64_t
types to std::abs() respectively. this triggers a warning
because std::abs() has no effect in this case. to rememdy this
we add template specialization for the execute() method of Abs
when its template paramter is U32 or U64.


Diffs (updated)
-----

src/arch/hsail/gen.py b31738224fb0eb259efc25f6d6efab5a962f29d0
src/gpu-compute/gpu_static_inst.hh b31738224fb0eb259efc25f6d6efab5a962f29d0

Diff: http://reviews.gem5.org/r/3446/diff/


Testing
-------


Thanks,

Tony Gutierrez
Andreas Hansson
2016-04-12 18:15:57 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3446/#review8191
-----------------------------------------------------------

Ship it!


Thanks!

- Andreas Hansson
Post by Tony Gutierrez
-----------------------------------------------------------
http://reviews.gem5.org/r/3446/
-----------------------------------------------------------
(Updated April 12, 2016, 5:20 p.m.)
Review request for Default.
Repository: gem5
Description
-------
Changeset 11443:3e0817967515
---------------------------
hsail,gpu-compute: fixes to appease clang++
fixes to appease clang++. tested on Ubuntu clang version
3.5.0-4ubuntu2~trusty2 (tags/RELEASE_350/final) (based on
LLVM 3.5.0)
1) the exec continuations in gpu_static_inst.hh were marked
as protected when they should be public. here we mark
them as public
2) the Abs instruction uses std::abs() in its execute method.
because Abs is templated, it can also operate on U32 and U64,
types, which cause Abs::execute() to pass uint32_t and uint64_t
types to std::abs() respectively. this triggers a warning
because std::abs() has no effect in this case. to rememdy this
we add template specialization for the execute() method of Abs
when its template paramter is U32 or U64.
Diffs
-----
src/arch/hsail/gen.py b31738224fb0eb259efc25f6d6efab5a962f29d0
src/gpu-compute/gpu_static_inst.hh b31738224fb0eb259efc25f6d6efab5a962f29d0
Diff: http://reviews.gem5.org/r/3446/diff/
Testing
-------
Thanks,
Tony Gutierrez
Tony Gutierrez
2016-04-13 15:03:48 UTC
Permalink
Post by Tony Gutierrez
Post by Andreas Hansson
Thanks!
There were a few more issues when I tried to run on zizzer, which has clang 3.6. These fixes allow the regressions to pass, disregarding the skipped eio tests.


- Tony


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3446/#review8191
-----------------------------------------------------------
Post by Tony Gutierrez
-----------------------------------------------------------
http://reviews.gem5.org/r/3446/
-----------------------------------------------------------
(Updated April 13, 2016, 7:58 a.m.)
Review request for Default.
Repository: gem5
Description
-------
Changeset 11443:c90af82c70e4
---------------------------
hsail,gpu-compute: fixes to appease clang++
Ubuntu clang version 3.5.0-4ubuntu2~trusty2
(tags/RELEASE_350/final) (based on LLVM 3.5.0)
Ubuntu clang version 3.6.0-2ubuntu1~trusty1
(tags/RELEASE_360/final) (based on LLVM 3.6.0)
1) the exec continuations in gpu_static_inst.hh were marked
as protected when they should be public. here we mark
them as public
2) the Abs instruction uses std::abs() in its execute method.
because Abs is templated, it can also operate on U32 and U64,
types, which cause Abs::execute() to pass uint32_t and uint64_t
types to std::abs() respectively. this triggers a warning
because std::abs() has no effect in this case. to rememdy this
we add template specialization for the execute() method of Abs
when its template paramter is U32 or U64.
3) Some potocols that utilize the code in cprintf.hh were missing
includes to BoolVec.hh, which defines operator<< for the BoolVec
type. This would cause issues when the generated code would try
to pass a BoolVec type to a method in cprintf.hh that used
operator<< on an instance of a BoolVec.
4) Surprise, clang doesn't like it when you clobber all the bits
tlb = new GpuTlbEntry[size];
std::memset(tlb, 0, sizeof(GpuTlbEntry) * size);
Let's use std::vector to track the TLB entries in the GpuTlb now...
5) There were a few variables used only in DPRINTFs, so we mark them
with M5_VAR_USED.
Diffs
-----
src/arch/hsail/gen.py b31738224fb0eb259efc25f6d6efab5a962f29d0
src/arch/x86/process.cc b31738224fb0eb259efc25f6d6efab5a962f29d0
src/gpu-compute/gpu_static_inst.hh b31738224fb0eb259efc25f6d6efab5a962f29d0
src/gpu-compute/gpu_tlb.hh b31738224fb0eb259efc25f6d6efab5a962f29d0
src/gpu-compute/gpu_tlb.cc b31738224fb0eb259efc25f6d6efab5a962f29d0
src/gpu-compute/hsail_code.hh b31738224fb0eb259efc25f6d6efab5a962f29d0
src/mem/slicc/symbols/StateMachine.py b31738224fb0eb259efc25f6d6efab5a962f29d0
Diff: http://reviews.gem5.org/r/3446/diff/
Testing
-------
Thanks,
Tony Gutierrez
Tony Gutierrez
2016-04-13 14:58:24 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3446/
-----------------------------------------------------------

(Updated April 13, 2016, 7:58 a.m.)


Review request for Default.


Repository: gem5


Description (updated)
-------

Changeset 11443:c90af82c70e4
---------------------------
hsail,gpu-compute: fixes to appease clang++

fixes to appease clang++. tested on:

Ubuntu clang version 3.5.0-4ubuntu2~trusty2
(tags/RELEASE_350/final) (based on LLVM 3.5.0)

Ubuntu clang version 3.6.0-2ubuntu1~trusty1
(tags/RELEASE_360/final) (based on LLVM 3.6.0)

the fixes address the following two issues:

1) the exec continuations in gpu_static_inst.hh were marked
as protected when they should be public. here we mark
them as public

2) the Abs instruction uses std::abs() in its execute method.
because Abs is templated, it can also operate on U32 and U64,
types, which cause Abs::execute() to pass uint32_t and uint64_t
types to std::abs() respectively. this triggers a warning
because std::abs() has no effect in this case. to rememdy this
we add template specialization for the execute() method of Abs
when its template paramter is U32 or U64.

3) Some potocols that utilize the code in cprintf.hh were missing
includes to BoolVec.hh, which defines operator<< for the BoolVec
type. This would cause issues when the generated code would try
to pass a BoolVec type to a method in cprintf.hh that used
operator<< on an instance of a BoolVec.

4) Surprise, clang doesn't like it when you clobber all the bits
in a newly allocated object. I.e., this code:

tlb = new GpuTlbEntry[size];
std::memset(tlb, 0, sizeof(GpuTlbEntry) * size);

Let's use std::vector to track the TLB entries in the GpuTlb now...

5) There were a few variables used only in DPRINTFs, so we mark them
with M5_VAR_USED.


Diffs (updated)
-----

src/arch/hsail/gen.py b31738224fb0eb259efc25f6d6efab5a962f29d0
src/arch/x86/process.cc b31738224fb0eb259efc25f6d6efab5a962f29d0
src/gpu-compute/gpu_static_inst.hh b31738224fb0eb259efc25f6d6efab5a962f29d0
src/gpu-compute/gpu_tlb.hh b31738224fb0eb259efc25f6d6efab5a962f29d0
src/gpu-compute/gpu_tlb.cc b31738224fb0eb259efc25f6d6efab5a962f29d0
src/gpu-compute/hsail_code.hh b31738224fb0eb259efc25f6d6efab5a962f29d0
src/mem/slicc/symbols/StateMachine.py b31738224fb0eb259efc25f6d6efab5a962f29d0

Diff: http://reviews.gem5.org/r/3446/diff/


Testing
-------


Thanks,

Tony Gutierrez
Tony Gutierrez
2016-04-13 15:09:10 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3446/
-----------------------------------------------------------

(Updated April 13, 2016, 8:09 a.m.)


Review request for Default.


Repository: gem5


Description (updated)
-------

Changeset 11443:c90af82c70e4
---------------------------
hsail,gpu-compute: fixes to appease clang++

fixes to appease clang++. tested on:

Ubuntu clang version 3.5.0-4ubuntu2~trusty2
(tags/RELEASE_350/final) (based on LLVM 3.5.0)

Ubuntu clang version 3.6.0-2ubuntu1~trusty1
(tags/RELEASE_360/final) (based on LLVM 3.6.0)

the fixes address the following five issues:

1) the exec continuations in gpu_static_inst.hh were marked
as protected when they should be public. here we mark
them as public

2) the Abs instruction uses std::abs() in its execute method.
because Abs is templated, it can also operate on U32 and U64,
types, which cause Abs::execute() to pass uint32_t and uint64_t
types to std::abs() respectively. this triggers a warning
because std::abs() has no effect in this case. to rememdy this
we add template specialization for the execute() method of Abs
when its template paramter is U32 or U64.

3) Some potocols that utilize the code in cprintf.hh were missing
includes to BoolVec.hh, which defines operator<< for the BoolVec
type. This would cause issues when the generated code would try
to pass a BoolVec type to a method in cprintf.hh that used
operator<< on an instance of a BoolVec.

4) Surprise, clang doesn't like it when you clobber all the bits
in a newly allocated object. I.e., this code:

tlb = new GpuTlbEntry[size];
std::memset(tlb, 0, sizeof(GpuTlbEntry) * size);

Let's use std::vector to track the TLB entries in the GpuTlb now...

5) There were a few variables used only in DPRINTFs, so we mark them
with M5_VAR_USED.


Diffs
-----

src/arch/hsail/gen.py b31738224fb0eb259efc25f6d6efab5a962f29d0
src/arch/x86/process.cc b31738224fb0eb259efc25f6d6efab5a962f29d0
src/gpu-compute/gpu_static_inst.hh b31738224fb0eb259efc25f6d6efab5a962f29d0
src/gpu-compute/gpu_tlb.hh b31738224fb0eb259efc25f6d6efab5a962f29d0
src/gpu-compute/gpu_tlb.cc b31738224fb0eb259efc25f6d6efab5a962f29d0
src/gpu-compute/hsail_code.hh b31738224fb0eb259efc25f6d6efab5a962f29d0
src/mem/slicc/symbols/StateMachine.py b31738224fb0eb259efc25f6d6efab5a962f29d0

Diff: http://reviews.gem5.org/r/3446/diff/


Testing
-------


Thanks,

Tony Gutierrez
Andreas Hansson
2016-04-13 16:24:31 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3446/#review8201
-----------------------------------------------------------


On the X86 side of things (but somewhat unrelated)...when compiled with clang-3.6 the following regression fails with a segfault:

build/X86/tests/opt/quick/fs/10.linux-boot/x86/linux/pc-simple-timing

Not sure why, but perhaps worth a look.

- Andreas Hansson
Post by Tony Gutierrez
-----------------------------------------------------------
http://reviews.gem5.org/r/3446/
-----------------------------------------------------------
(Updated April 13, 2016, 3:09 p.m.)
Review request for Default.
Repository: gem5
Description
-------
Changeset 11443:c90af82c70e4
---------------------------
hsail,gpu-compute: fixes to appease clang++
Ubuntu clang version 3.5.0-4ubuntu2~trusty2
(tags/RELEASE_350/final) (based on LLVM 3.5.0)
Ubuntu clang version 3.6.0-2ubuntu1~trusty1
(tags/RELEASE_360/final) (based on LLVM 3.6.0)
1) the exec continuations in gpu_static_inst.hh were marked
as protected when they should be public. here we mark
them as public
2) the Abs instruction uses std::abs() in its execute method.
because Abs is templated, it can also operate on U32 and U64,
types, which cause Abs::execute() to pass uint32_t and uint64_t
types to std::abs() respectively. this triggers a warning
because std::abs() has no effect in this case. to rememdy this
we add template specialization for the execute() method of Abs
when its template paramter is U32 or U64.
3) Some potocols that utilize the code in cprintf.hh were missing
includes to BoolVec.hh, which defines operator<< for the BoolVec
type. This would cause issues when the generated code would try
to pass a BoolVec type to a method in cprintf.hh that used
operator<< on an instance of a BoolVec.
4) Surprise, clang doesn't like it when you clobber all the bits
tlb = new GpuTlbEntry[size];
std::memset(tlb, 0, sizeof(GpuTlbEntry) * size);
Let's use std::vector to track the TLB entries in the GpuTlb now...
5) There were a few variables used only in DPRINTFs, so we mark them
with M5_VAR_USED.
Diffs
-----
src/arch/hsail/gen.py b31738224fb0eb259efc25f6d6efab5a962f29d0
src/arch/x86/process.cc b31738224fb0eb259efc25f6d6efab5a962f29d0
src/gpu-compute/gpu_static_inst.hh b31738224fb0eb259efc25f6d6efab5a962f29d0
src/gpu-compute/gpu_tlb.hh b31738224fb0eb259efc25f6d6efab5a962f29d0
src/gpu-compute/gpu_tlb.cc b31738224fb0eb259efc25f6d6efab5a962f29d0
src/gpu-compute/hsail_code.hh b31738224fb0eb259efc25f6d6efab5a962f29d0
src/mem/slicc/symbols/StateMachine.py b31738224fb0eb259efc25f6d6efab5a962f29d0
Diff: http://reviews.gem5.org/r/3446/diff/
Testing
-------
Thanks,
Tony Gutierrez
Tony Gutierrez
2016-04-13 16:32:42 UTC
Permalink
Post by Tony Gutierrez
Post by Andreas Hansson
build/X86/tests/opt/quick/fs/10.linux-boot/x86/linux/pc-simple-timing
Not sure why, but perhaps worth a look.
That regression passes for me with zizzer's (usr/bin/clang++-3.6) compiler and this patch applied:

"build/X86/tests/opt/quick/fs/10.linux-boot/x86/linux/pc-simple-timing passed."

Not sure if that is the compiler used to run the regressions, because there is no symlink in /usr/bin and there are no other versions of clang I can see in any of the canonical paths. So I manually set CC/CXX to this version of clang on zizzer.


- Tony


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3446/#review8201
-----------------------------------------------------------
Post by Tony Gutierrez
-----------------------------------------------------------
http://reviews.gem5.org/r/3446/
-----------------------------------------------------------
(Updated April 13, 2016, 8:09 a.m.)
Review request for Default.
Repository: gem5
Description
-------
Changeset 11443:c90af82c70e4
---------------------------
hsail,gpu-compute: fixes to appease clang++
Ubuntu clang version 3.5.0-4ubuntu2~trusty2
(tags/RELEASE_350/final) (based on LLVM 3.5.0)
Ubuntu clang version 3.6.0-2ubuntu1~trusty1
(tags/RELEASE_360/final) (based on LLVM 3.6.0)
1) the exec continuations in gpu_static_inst.hh were marked
as protected when they should be public. here we mark
them as public
2) the Abs instruction uses std::abs() in its execute method.
because Abs is templated, it can also operate on U32 and U64,
types, which cause Abs::execute() to pass uint32_t and uint64_t
types to std::abs() respectively. this triggers a warning
because std::abs() has no effect in this case. to rememdy this
we add template specialization for the execute() method of Abs
when its template paramter is U32 or U64.
3) Some potocols that utilize the code in cprintf.hh were missing
includes to BoolVec.hh, which defines operator<< for the BoolVec
type. This would cause issues when the generated code would try
to pass a BoolVec type to a method in cprintf.hh that used
operator<< on an instance of a BoolVec.
4) Surprise, clang doesn't like it when you clobber all the bits
tlb = new GpuTlbEntry[size];
std::memset(tlb, 0, sizeof(GpuTlbEntry) * size);
Let's use std::vector to track the TLB entries in the GpuTlb now...
5) There were a few variables used only in DPRINTFs, so we mark them
with M5_VAR_USED.
Diffs
-----
src/arch/hsail/gen.py b31738224fb0eb259efc25f6d6efab5a962f29d0
src/arch/x86/process.cc b31738224fb0eb259efc25f6d6efab5a962f29d0
src/gpu-compute/gpu_static_inst.hh b31738224fb0eb259efc25f6d6efab5a962f29d0
src/gpu-compute/gpu_tlb.hh b31738224fb0eb259efc25f6d6efab5a962f29d0
src/gpu-compute/gpu_tlb.cc b31738224fb0eb259efc25f6d6efab5a962f29d0
src/gpu-compute/hsail_code.hh b31738224fb0eb259efc25f6d6efab5a962f29d0
src/mem/slicc/symbols/StateMachine.py b31738224fb0eb259efc25f6d6efab5a962f29d0
Diff: http://reviews.gem5.org/r/3446/diff/
Testing
-------
Thanks,
Tony Gutierrez
Andreas Hansson
2016-04-13 16:37:18 UTC
Permalink
Post by Tony Gutierrez
Post by Andreas Hansson
build/X86/tests/opt/quick/fs/10.linux-boot/x86/linux/pc-simple-timing
Not sure why, but perhaps worth a look.
"build/X86/tests/opt/quick/fs/10.linux-boot/x86/linux/pc-simple-timing passed."
Not sure if that is the compiler used to run the regressions, because there is no symlink in /usr/bin and there are no other versions of clang I can see in any of the canonical paths. So I manually set CC/CXX to this version of clang on zizzer.
Ok, let me see if I can reproduce on another system.


- Andreas


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3446/#review8201
-----------------------------------------------------------
Post by Tony Gutierrez
-----------------------------------------------------------
http://reviews.gem5.org/r/3446/
-----------------------------------------------------------
(Updated April 13, 2016, 3:09 p.m.)
Review request for Default.
Repository: gem5
Description
-------
Changeset 11443:c90af82c70e4
---------------------------
hsail,gpu-compute: fixes to appease clang++
Ubuntu clang version 3.5.0-4ubuntu2~trusty2
(tags/RELEASE_350/final) (based on LLVM 3.5.0)
Ubuntu clang version 3.6.0-2ubuntu1~trusty1
(tags/RELEASE_360/final) (based on LLVM 3.6.0)
1) the exec continuations in gpu_static_inst.hh were marked
as protected when they should be public. here we mark
them as public
2) the Abs instruction uses std::abs() in its execute method.
because Abs is templated, it can also operate on U32 and U64,
types, which cause Abs::execute() to pass uint32_t and uint64_t
types to std::abs() respectively. this triggers a warning
because std::abs() has no effect in this case. to rememdy this
we add template specialization for the execute() method of Abs
when its template paramter is U32 or U64.
3) Some potocols that utilize the code in cprintf.hh were missing
includes to BoolVec.hh, which defines operator<< for the BoolVec
type. This would cause issues when the generated code would try
to pass a BoolVec type to a method in cprintf.hh that used
operator<< on an instance of a BoolVec.
4) Surprise, clang doesn't like it when you clobber all the bits
tlb = new GpuTlbEntry[size];
std::memset(tlb, 0, sizeof(GpuTlbEntry) * size);
Let's use std::vector to track the TLB entries in the GpuTlb now...
5) There were a few variables used only in DPRINTFs, so we mark them
with M5_VAR_USED.
Diffs
-----
src/arch/hsail/gen.py b31738224fb0eb259efc25f6d6efab5a962f29d0
src/arch/x86/process.cc b31738224fb0eb259efc25f6d6efab5a962f29d0
src/gpu-compute/gpu_static_inst.hh b31738224fb0eb259efc25f6d6efab5a962f29d0
src/gpu-compute/gpu_tlb.hh b31738224fb0eb259efc25f6d6efab5a962f29d0
src/gpu-compute/gpu_tlb.cc b31738224fb0eb259efc25f6d6efab5a962f29d0
src/gpu-compute/hsail_code.hh b31738224fb0eb259efc25f6d6efab5a962f29d0
src/mem/slicc/symbols/StateMachine.py b31738224fb0eb259efc25f6d6efab5a962f29d0
Diff: http://reviews.gem5.org/r/3446/diff/
Testing
-------
Thanks,
Tony Gutierrez
Tony Gutierrez
2016-09-16 18:59:47 UTC
Permalink
Post by Tony Gutierrez
Post by Andreas Hansson
build/X86/tests/opt/quick/fs/10.linux-boot/x86/linux/pc-simple-timing
Not sure why, but perhaps worth a look.
"build/X86/tests/opt/quick/fs/10.linux-boot/x86/linux/pc-simple-timing passed."
Not sure if that is the compiler used to run the regressions, because there is no symlink in /usr/bin and there are no other versions of clang I can see in any of the canonical paths. So I manually set CC/CXX to this version of clang on zizzer.
Ok, let me see if I can reproduce on another system.
I think some of the issues this patch addresses persist. Do you have any further issues with this patch?


- Tony


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3446/#review8201
-----------------------------------------------------------
Post by Tony Gutierrez
-----------------------------------------------------------
http://reviews.gem5.org/r/3446/
-----------------------------------------------------------
(Updated April 13, 2016, 8:09 a.m.)
Review request for Default.
Repository: gem5
Description
-------
Changeset 11443:c90af82c70e4
---------------------------
hsail,gpu-compute: fixes to appease clang++
Ubuntu clang version 3.5.0-4ubuntu2~trusty2
(tags/RELEASE_350/final) (based on LLVM 3.5.0)
Ubuntu clang version 3.6.0-2ubuntu1~trusty1
(tags/RELEASE_360/final) (based on LLVM 3.6.0)
1) the exec continuations in gpu_static_inst.hh were marked
as protected when they should be public. here we mark
them as public
2) the Abs instruction uses std::abs() in its execute method.
because Abs is templated, it can also operate on U32 and U64,
types, which cause Abs::execute() to pass uint32_t and uint64_t
types to std::abs() respectively. this triggers a warning
because std::abs() has no effect in this case. to rememdy this
we add template specialization for the execute() method of Abs
when its template paramter is U32 or U64.
3) Some potocols that utilize the code in cprintf.hh were missing
includes to BoolVec.hh, which defines operator<< for the BoolVec
type. This would cause issues when the generated code would try
to pass a BoolVec type to a method in cprintf.hh that used
operator<< on an instance of a BoolVec.
4) Surprise, clang doesn't like it when you clobber all the bits
tlb = new GpuTlbEntry[size];
std::memset(tlb, 0, sizeof(GpuTlbEntry) * size);
Let's use std::vector to track the TLB entries in the GpuTlb now...
5) There were a few variables used only in DPRINTFs, so we mark them
with M5_VAR_USED.
Diffs
-----
src/arch/hsail/gen.py b31738224fb0eb259efc25f6d6efab5a962f29d0
src/arch/x86/process.cc b31738224fb0eb259efc25f6d6efab5a962f29d0
src/gpu-compute/gpu_static_inst.hh b31738224fb0eb259efc25f6d6efab5a962f29d0
src/gpu-compute/gpu_tlb.hh b31738224fb0eb259efc25f6d6efab5a962f29d0
src/gpu-compute/gpu_tlb.cc b31738224fb0eb259efc25f6d6efab5a962f29d0
src/gpu-compute/hsail_code.hh b31738224fb0eb259efc25f6d6efab5a962f29d0
src/mem/slicc/symbols/StateMachine.py b31738224fb0eb259efc25f6d6efab5a962f29d0
Diff: http://reviews.gem5.org/r/3446/diff/
Testing
-------
Thanks,
Tony Gutierrez
Joe Gross
2016-10-24 21:24:26 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3446/#review8978
-----------------------------------------------------------

Ship it!


This fixed a bunch of ubsan warnings with gcc6.1.0 for me.

- Joe Gross
Post by Tony Gutierrez
-----------------------------------------------------------
http://reviews.gem5.org/r/3446/
-----------------------------------------------------------
(Updated April 13, 2016, 10:09 a.m.)
Review request for Default.
Repository: gem5
Description
-------
Changeset 11443:c90af82c70e4
---------------------------
hsail,gpu-compute: fixes to appease clang++
Ubuntu clang version 3.5.0-4ubuntu2~trusty2
(tags/RELEASE_350/final) (based on LLVM 3.5.0)
Ubuntu clang version 3.6.0-2ubuntu1~trusty1
(tags/RELEASE_360/final) (based on LLVM 3.6.0)
1) the exec continuations in gpu_static_inst.hh were marked
as protected when they should be public. here we mark
them as public
2) the Abs instruction uses std::abs() in its execute method.
because Abs is templated, it can also operate on U32 and U64,
types, which cause Abs::execute() to pass uint32_t and uint64_t
types to std::abs() respectively. this triggers a warning
because std::abs() has no effect in this case. to rememdy this
we add template specialization for the execute() method of Abs
when its template paramter is U32 or U64.
3) Some potocols that utilize the code in cprintf.hh were missing
includes to BoolVec.hh, which defines operator<< for the BoolVec
type. This would cause issues when the generated code would try
to pass a BoolVec type to a method in cprintf.hh that used
operator<< on an instance of a BoolVec.
4) Surprise, clang doesn't like it when you clobber all the bits
tlb = new GpuTlbEntry[size];
std::memset(tlb, 0, sizeof(GpuTlbEntry) * size);
Let's use std::vector to track the TLB entries in the GpuTlb now...
5) There were a few variables used only in DPRINTFs, so we mark them
with M5_VAR_USED.
Diffs
-----
src/arch/hsail/gen.py b31738224fb0eb259efc25f6d6efab5a962f29d0
src/arch/x86/process.cc b31738224fb0eb259efc25f6d6efab5a962f29d0
src/gpu-compute/gpu_static_inst.hh b31738224fb0eb259efc25f6d6efab5a962f29d0
src/gpu-compute/gpu_tlb.hh b31738224fb0eb259efc25f6d6efab5a962f29d0
src/gpu-compute/gpu_tlb.cc b31738224fb0eb259efc25f6d6efab5a962f29d0
src/gpu-compute/hsail_code.hh b31738224fb0eb259efc25f6d6efab5a962f29d0
src/mem/slicc/symbols/StateMachine.py b31738224fb0eb259efc25f6d6efab5a962f29d0
Diff: http://reviews.gem5.org/r/3446/diff/
Testing
-------
Thanks,
Tony Gutierrez
Loading...