Fix DEADCODE Coverity issue for acl_context_test

[IlanTruanovsky]

The two issues were:

1. apitype only ever had a value of 0. The for loop it is initialized in exits once apitype >= 1. I'm not sure if this is a typo by the original code writer? I tried having it take on values 0 and 1 (as similar code does in other parts of this file), but it errored out. I could try to debug this further if this is seen as the proper solution. For now though, I just removed the for loop with the apitype variable that looped over nothing.
2. fixed_val was never used and never modified.

Fixes:

test/acl_context_test.cpp:1116:9:
  Type: Logically dead code (DEADCODE)

test/acl_context_test.cpp:1111:9:
  cond_const: Condition "apitype", taking false branch. Now the value of "apitype" is equal to 0.
test/acl_context_test.cpp:1117:13:
  const: At condition "apitype", the value of "apitype" must be equal to 0.
test/acl_context_test.cpp:1116:9:
  dead_error_condition: The condition "apitype" cannot be true.
test/acl_context_test.cpp:1116:9:
  dead_error_line: Execution cannot reach the expression "clCreateContextFromType(props, 4294967295UL, notify_me, this, &status)" inside this statement: "context = (apitype ? clCrea...".

test/acl_context_test.cpp:1125:11:
  Type: Logically dead code (DEADCODE)

test/acl_context_test.cpp:1103:9:
  assignment: Assigning: "fixed_val" = "0L".
test/acl_context_test.cpp:1124:13:
  const: At condition "fixed_val != 0L", the value of "fixed_val" must be equal to 0.
test/acl_context_test.cpp:1124:9:
  dead_error_condition: The condition "fixed_val != 0L" cannot be true.
test/acl_context_test.cpp:1125:11:
  dead_error_line: Execution cannot reach this statement: "if (!envvals[ienv].valid) {...".

[IlanTruanovsky]

1. apitype only ever had a value of 0. The for loop it is initialized in exits once apitype >= 1. I'm not sure if this is a typo by the original code writer? I tried having it take on values 0 and 1 (as similar code does in other parts of this file), but it errored out. I could try to debug this further if this is seen as the proper solution. For now though, I just removed the for loop with the apitype variable that looped over nothing.

@pcolberg What are your thoughts on this? Do you think we should remove the loop completely or try to fix the case that failed when apitype = 1?

[pcolberg]

1. apitype only ever had a value of 0. The for loop it is initialized in exits once apitype >= 1. I'm not sure if this is a typo by the original code writer? I tried having it take on values 0 and 1 (as similar code does in other parts of this file), but it errored out. I could try to debug this further if this is seen as the proper solution. For now though, I just removed the for loop with the apitype variable that looped over nothing.

@pcolberg What are your thoughts on this? Do you think we should remove the loop completely or try to fix the case that failed when apitype = 1?

This looks like a typo; there is another occurrence where the loop correctly tests both clCreateContext*() variants. What error in particular do you get here?

fpga-runtime-for-opencl/test/acl_context_test.cpp

Lines 975 to 999 in 5a71e64

	for (int apitype = 0; apitype < 2; apitype++) {
	for (int i = 0; i < 2; i++) { // without/with platform.
	for (int j = -1; j <= CL_CONTEXT_COMPILER_MODE_NUM_MODES_INTELFPGA;
	j++) {
	int idx = 0;
	if (i) {
	props[idx++] = CL_CONTEXT_PLATFORM;
	props[idx++] = (cl_context_properties)m_platform;
	ACL_LOCKED(acl_print_debug_msg(" set platform %p\n", m_platform));
	}
	if (j >= 0) {
	props[idx++] = CL_CONTEXT_COMPILER_MODE_INTELFPGA;
	props[idx++] = (cl_context_properties)j;
	}
	props[idx++] = 0;
	ACL_LOCKED(
	acl_print_debug_msg(" create with env str '%s' i %d j %d\n",
	envvals[ienv].str, i, j));
	cl_int status = CL_INVALID_VALUE;
	cl_context context =
	apitype ? clCreateContextFromType(props, CL_DEVICE_TYPE_ALL, 0, 0,
	&status)
	: clCreateContext(props, 1, m_device, notify_me, this,
	&status);

[IlanTruanovsky]

The clCreateContextFromType returns a status of -33, which is the CL_INVALID_DEVICE error code.

Specifically, this is returned by line 899 of acl_context.cpp. It detects that there are 5 devices present, and the 6th device is absent. I'm looking more into this.

[IlanTruanovsky]

I found that this error only occurs when ienv = 1 and after having set CL_CONTEXT_OFFLINE_DEVICE_INTELFPGA=+a10_ref_small in the environment. I haven't looked over the source code enough yet to see what the implications of this are, but I'll be taking a closer look later.

[IlanTruanovsky]

I've taken a good look at the code now and I'm going to try and explain what is happening in the my own words. For the problematic iteration (i.e. ienv = 1, apitype = 1, i = 0):

1. First, we set the environment to contain L_CONTEXT_OFFLINE_DEVICE_INTELFPGA=+a10_ref_small (that happens here).
2. Then, we initialize our 5 accelerators that we use in the test. Since we have L_CONTEXT_OFFLINE_DEVICE_INTELFPGA=+a10_ref_small in our environment, this is equivalent to adding a 6th offline accelerator to our system. The initialization step happens here.
3. We then try to get our context by calling clCreateContextFromType. Note that we use CL_DEVICE_TYPE_ALL meaning we want all device types included in the context (i.e. doesn't matter if the devices are GPUs, CPUs, etc).
4. Inside the clCreateContextFromType call, we get our desired set of devices by calling clGetDeviceIDs and pass them to l_finalize_context. The devices that we get from clGetDeviceIDs include the offline device we added through the environment variable.
5. l_finalize_context calls l_init_context_with_devices with the same devices.
6. Finally, l_init_context_with_devices correctly errors out since we have a combination of present and absent (offline) devices.

With all that said...

I think it returning CL_INVALID_DEVICE is correct, and I think I should update the test to reflect this.

What do you think, @pcolberg?

[pcolberg]

I think it returning CL_INVALID_DEVICE is correct, and I think I should update the test to reflect this.

Yes, that is good. Thanks for the clear, detailed analysis; please be sure to save it in the commit message.