Skip to content

Commit 59e9579

Browse files
scemamascemama
authored
Merge pull request #13 from scemama/copilot/improve-code-quality
[WIP] Improve code quality and add CI workflow
2 parents a2eb368 + 1de4197 commit 59e9579

4 files changed

Lines changed: 176 additions & 18 deletions

File tree

‎.github/workflows/ci.yml‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
name: CI
2+
3+
on:
4+
push:
5+
branches: [ main, master ]
6+
pull_request:
7+
branches: [ main, master ]
8+
workflow_dispatch:
9+
10+
permissions:
11+
contents: read
12+
13+
jobs:
14+
build-and-test:
15+
runs-on: ubuntu-latest
16+
strategy:
17+
matrix:
18+
compiler:
19+
- cc: gcc
20+
fc: gfortran
21+
- cc: clang
22+
fc: gfortran
23+
24+
steps:
25+
- name: Checkout repository
26+
uses: actions/checkout@v4
27+
28+
- name: Install dependencies
29+
run: |
30+
sudo apt-get update -qq
31+
sudo apt-get install -y \
32+
autoconf \
33+
automake \
34+
libtool \
35+
${{ matrix.compiler.cc }} \
36+
${{ matrix.compiler.fc }} \
37+
libopenblas-dev
38+
39+
- name: Generate configure script
40+
run: ./autogen.sh
41+
42+
- name: Configure (CPU-only version)
43+
run: |
44+
./configure \
45+
CC=${{ matrix.compiler.cc }} \
46+
FC=${{ matrix.compiler.fc }} \
47+
--disable-nvidia \
48+
--disable-amd
49+
50+
- name: Build
51+
run: make -j$(nproc)
52+
53+
- name: Run tests
54+
run: make check
55+
56+
- name: Display test results on failure
57+
if: failure()
58+
run: |
59+
if [ -f tests/test-suite.log ]; then
60+
echo "=== Test Suite Log ==="
61+
cat tests/test-suite.log
62+
fi
63+
for log in tests/*.log; do
64+
if [ -f "$log" ]; then
65+
echo "=== $log ==="
66+
cat "$log"
67+
fi
68+
done

‎src/gpu_amd.c‎

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,15 @@ int gpu_ndevices() {
2121
}
2222

2323
void gpu_set_device(int32_t igpu) {
24+
if (igpu < 0) {
25+
fprintf(stderr, "gpu_set_device: invalid device index %d (must be >= 0)\n", igpu);
26+
return;
27+
}
28+
int ngpus = gpu_ndevices();
29+
if (igpu >= ngpus) {
30+
fprintf(stderr, "gpu_set_device: invalid device index %d (only %d devices available)\n", igpu, ngpus);
31+
return;
32+
}
2433
hipError_t rc = hipSetDevice((int)igpu);
2534
if (rc != hipSuccess) {
2635
fprintf(stderr,"hipSetDevice(%d) failed: %s\n", igpu, hipGetErrorString(rc));
@@ -39,6 +48,11 @@ void gpu_get_memory(size_t* free, size_t* total) {
3948
/* Allocation functions */
4049

4150
void gpu_allocate(void** ptr, const int64_t size) {
51+
if (ptr == NULL) {
52+
fprintf(stderr, "gpu_allocate: ptr argument is NULL\n");
53+
return;
54+
}
55+
4256
size_t free, total;
4357
hipError_t rc = hipMemGetInfo( &free, &total );
4458
if (rc != hipSuccess) {
@@ -54,7 +68,9 @@ void gpu_allocate(void** ptr, const int64_t size) {
5468
}
5569

5670
void gpu_deallocate(void** ptr) {
57-
assert (*ptr != NULL);
71+
if (ptr == NULL || *ptr == NULL) {
72+
return;
73+
}
5874
hipError_t rc = hipFree(*ptr);
5975
if (rc != hipSuccess) {
6076
fprintf(stderr,"hipFree failed: %s\n", hipGetErrorString(rc));
@@ -71,6 +87,10 @@ void gpu_free(void** ptr) {
7187
/* Memory transfer functions */
7288

7389
void gpu_upload(const void* cpu_ptr, void* gpu_ptr, const int64_t n) {
90+
if (cpu_ptr == NULL || gpu_ptr == NULL) {
91+
fprintf(stderr, "gpu_upload: NULL pointer argument\n");
92+
return;
93+
}
7494
hipError_t rc = hipMemcpy (gpu_ptr, cpu_ptr, n, hipMemcpyHostToDevice);
7595
if (rc != hipSuccess) {
7696
fprintf(stderr,"hipMemcpy (upload) failed: %s\n", hipGetErrorString(rc));
@@ -79,6 +99,10 @@ void gpu_upload(const void* cpu_ptr, void* gpu_ptr, const int64_t n) {
7999
}
80100

81101
void gpu_download(const void* gpu_ptr, void* cpu_ptr, const int64_t n) {
102+
if (gpu_ptr == NULL || cpu_ptr == NULL) {
103+
fprintf(stderr, "gpu_download: NULL pointer argument\n");
104+
return;
105+
}
82106
hipError_t rc = hipMemcpy (cpu_ptr, gpu_ptr, n, hipMemcpyDeviceToHost);
83107
if (rc != hipSuccess) {
84108
fprintf(stderr,"hipMemcpy (download) failed: %s\n", hipGetErrorString(rc));
@@ -87,6 +111,10 @@ void gpu_download(const void* gpu_ptr, void* cpu_ptr, const int64_t n) {
87111
}
88112

89113
void gpu_copy(const void* gpu_ptr_src, void* gpu_ptr_dest, const int64_t n) {
114+
if (gpu_ptr_src == NULL || gpu_ptr_dest == NULL) {
115+
fprintf(stderr, "gpu_copy: NULL pointer argument\n");
116+
return;
117+
}
90118
hipError_t rc = hipMemcpy (gpu_ptr_dest, gpu_ptr_src, n, hipMemcpyDeviceToDevice);
91119
if (rc != hipSuccess) {
92120
fprintf(stderr,"hipMemcpy (copy) failed: %s\n", hipGetErrorString(rc));
@@ -143,11 +171,15 @@ void gpu_stream_synchronize(void* stream) {
143171
/* BLAS functions */
144172

145173
void gpu_blas_create(hipblasHandle_t* ptr) {
174+
if (ptr == NULL) {
175+
fprintf(stderr, "gpu_blas_create: ptr argument is NULL\n");
176+
return;
177+
}
146178
hipblasStatus_t rc = hipblasCreate(ptr);
147179
if (rc != HIPBLAS_STATUS_SUCCESS) {
148180
fprintf(stderr,"hipblasCreate failed\n");
181+
assert (rc == HIPBLAS_STATUS_SUCCESS);
149182
}
150-
assert (rc == HIPBLAS_STATUS_SUCCESS);
151183
}
152184

153185

‎src/gpu_cpu.c‎

Lines changed: 41 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,13 +24,21 @@ void gpu_get_memory(size_t* free, size_t* total) {
2424
/* Allocation functions */
2525

2626
void gpu_allocate(void** ptr, const int64_t n) {
27+
if (ptr == NULL) {
28+
fprintf(stderr, "gpu_allocate: ptr argument is NULL\n");
29+
return;
30+
}
2731
*ptr = malloc((size_t) n);
2832
if (*ptr == NULL) {
29-
perror("Allocation failed");
33+
perror("gpu_allocate: malloc failed");
34+
assert(*ptr != NULL);
3035
}
3136
}
3237

3338
void gpu_deallocate(void** ptr) {
39+
if (ptr == NULL || *ptr == NULL) {
40+
return;
41+
}
3442
free(*ptr);
3543
*ptr = NULL;
3644
}
@@ -43,14 +51,26 @@ void gpu_free(void** ptr) {
4351
/* Memory transfer functions */
4452

4553
void gpu_upload(const void* cpu_ptr, void* gpu_ptr, const int64_t n) {
54+
if (cpu_ptr == NULL || gpu_ptr == NULL) {
55+
fprintf(stderr, "gpu_upload: NULL pointer argument\n");
56+
return;
57+
}
4658
memcpy(gpu_ptr, cpu_ptr, n);
4759
}
4860

4961
void gpu_download(const void* gpu_ptr, void* cpu_ptr, const int64_t n) {
62+
if (gpu_ptr == NULL || cpu_ptr == NULL) {
63+
fprintf(stderr, "gpu_download: NULL pointer argument\n");
64+
return;
65+
}
5066
memcpy(cpu_ptr, gpu_ptr, n);
5167
}
5268

5369
void gpu_copy(const void* gpu_ptr_src, void* gpu_ptr_dest, const int64_t n) {
70+
if (gpu_ptr_src == NULL || gpu_ptr_dest == NULL) {
71+
fprintf(stderr, "gpu_copy: NULL pointer argument\n");
72+
return;
73+
}
5474
memcpy(gpu_ptr_dest, gpu_ptr_src, n);
5575
}
5676

@@ -81,6 +101,20 @@ void gpu_stream_synchronize(void* stream) {
81101

82102
/* BLAS functions */
83103

104+
/**
105+
* @brief Check if an int64_t value can be safely converted to int32_t
106+
* @param value The int64_t value to check
107+
* @param name The name of the parameter (for error messages)
108+
* @return true if overflow would occur, false otherwise
109+
*/
110+
static inline bool check_int32_overflow(int64_t value, const char* name) {
111+
if (value > INT32_MAX || value < INT32_MIN) {
112+
fprintf(stderr, "Integer overflow: %s value %lld exceeds int32_t range\n", name, (long long)value);
113+
return true;
114+
}
115+
return false;
116+
}
117+
84118
void gpu_blas_create(void** handle) {
85119
*handle = (void*) malloc(sizeof(char));
86120
}
@@ -105,9 +139,12 @@ void gpu_ddot(void* handle, const int64_t n, const double* x, const int64_t incx
105139
incy_ = (int32_t) incy;
106140

107141
/* Check for integer overflows */
108-
assert ( (int64_t) n_ == n );
109-
assert ( (int64_t) incx_ == incx);
110-
assert ( (int64_t) incy_ == incy);
142+
if (check_int32_overflow(n, "n") ||
143+
check_int32_overflow(incx, "incx") ||
144+
check_int32_overflow(incy, "incy")) {
145+
*result = 0.0;
146+
return;
147+
}
111148

112149
*result = ddot_(&n_, x, &incx_, y, &incy_);
113150
}

‎src/gpu_nvidia.c‎

Lines changed: 33 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,15 @@ int gpu_ndevices() {
1818
}
1919

2020
void gpu_set_device(int32_t igpu) {
21+
if (igpu < 0) {
22+
fprintf(stderr, "gpu_set_device: invalid device index %d (must be >= 0)\n", igpu);
23+
return;
24+
}
25+
int ngpus = gpu_ndevices();
26+
if (igpu >= ngpus) {
27+
fprintf(stderr, "gpu_set_device: invalid device index %d (only %d devices available)\n", igpu, ngpus);
28+
return;
29+
}
2130
cudaError_t rc = cudaSetDevice((int) igpu);
2231
if (rc != cudaSuccess) {
2332
fprintf(stderr,"cudaSetDevice(%d) failed: %s\n", igpu, cudaGetErrorString(rc));
@@ -36,29 +45,29 @@ void gpu_get_memory(size_t* free, size_t* total) {
3645
/* Allocation functions */
3746

3847
void gpu_allocate(void** ptr, const int64_t size) {
48+
if (ptr == NULL) {
49+
fprintf(stderr, "gpu_allocate: ptr argument is NULL\n");
50+
return;
51+
}
52+
3953
size_t free, total;
4054
cudaError_t rc = cudaMemGetInfo( &free, &total );
4155
if (rc != cudaSuccess) {
4256
free = INT64_MAX;
4357
}
4458

45-
// rc = cudaMallocManaged(ptr, size, cudaMemAttachGlobal);
46-
rc= cudaMalloc(ptr, size);
59+
rc = cudaMalloc(ptr, size);
4760

48-
// /* Use managed memory if it does not fit on the GPU */
49-
// if (size < free && size < total/2) {
50-
// rc= cudaMalloc(ptr, size);
51-
// } else {
52-
// rc = cudaMallocManaged(ptr, size, cudaMemAttachGlobal);
53-
// }
5461
if (rc != cudaSuccess) {
55-
fprintf(stderr,"cudaMallocManaged failed: %s\n", cudaGetErrorString(rc));
62+
fprintf(stderr,"cudaMalloc failed: %s\n", cudaGetErrorString(rc));
5663
assert (rc == cudaSuccess);
5764
}
5865
}
5966

6067
void gpu_deallocate(void** ptr) {
61-
assert (*ptr != NULL);
68+
if (ptr == NULL || *ptr == NULL) {
69+
return;
70+
}
6271
cudaFree(*ptr);
6372
*ptr = NULL;
6473
}
@@ -71,6 +80,10 @@ void gpu_free(void** ptr) {
7180
/* Memory transfer functions */
7281

7382
void gpu_upload(const void* cpu_ptr, void* gpu_ptr, const int64_t n) {
83+
if (cpu_ptr == NULL || gpu_ptr == NULL) {
84+
fprintf(stderr, "gpu_upload: NULL pointer argument\n");
85+
return;
86+
}
7487
cudaError_t rc = cudaMemcpy (gpu_ptr, cpu_ptr, n, cudaMemcpyHostToDevice);
7588
if (rc != cudaSuccess) {
7689
fprintf(stderr,"cudaMemcpy (upload) failed: %s\n", cudaGetErrorString(rc));
@@ -79,6 +92,10 @@ void gpu_upload(const void* cpu_ptr, void* gpu_ptr, const int64_t n) {
7992
}
8093

8194
void gpu_download(const void* gpu_ptr, void* cpu_ptr, const int64_t n) {
95+
if (gpu_ptr == NULL || cpu_ptr == NULL) {
96+
fprintf(stderr, "gpu_download: NULL pointer argument\n");
97+
return;
98+
}
8299
cudaError_t rc = cudaMemcpy (cpu_ptr, gpu_ptr, n, cudaMemcpyDeviceToHost);
83100
if (rc != cudaSuccess) {
84101
fprintf(stderr,"cudaMemcpy (download) failed: %s\n", cudaGetErrorString(rc));
@@ -87,6 +104,10 @@ void gpu_download(const void* gpu_ptr, void* cpu_ptr, const int64_t n) {
87104
}
88105

89106
void gpu_copy(const void* gpu_ptr_src, void* gpu_ptr_dest, const int64_t n) {
107+
if (gpu_ptr_src == NULL || gpu_ptr_dest == NULL) {
108+
fprintf(stderr, "gpu_copy: NULL pointer argument\n");
109+
return;
110+
}
90111
cudaError_t rc = cudaMemcpy (gpu_ptr_dest, gpu_ptr_src, n, cudaMemcpyDeviceToDevice);
91112
if (rc != cudaSuccess) {
92113
fprintf(stderr,"cudaMemcpy (copy) failed: %s\n", cudaGetErrorString(rc));
@@ -118,8 +139,8 @@ void gpu_stream_destroy(cudaStream_t* ptr) {
118139
void gpu_set_stream(cublasHandle_t handle, cudaStream_t stream) {
119140
cublasStatus_t rc = cublasSetStream(handle, stream);
120141
if (rc != CUBLAS_STATUS_SUCCESS) {
121-
fprintf(stderr,"cudaSetStream failed\n");
122-
assert (rc == cudaSuccess);
142+
fprintf(stderr,"cublasSetStream failed\n");
143+
assert (rc == CUBLAS_STATUS_SUCCESS);
123144
}
124145
}
125146

0 commit comments

Comments
 (0)